tfiala added a comment.
> On OS X there was at least one test that would hang each run, so I didn't get
> any kind of real timing numbers there since everything was always maxed out
> by the hanging test.
Well wonders abound. On my OS X setup, I am not seeing the hanging tests with
TOT from
tfiala closed this revision.
tfiala added a comment.
Committed here:
Sendingtest/dosep.py
Sendingtest/dotest.py
Sendingtest/dotest_args.py
Transmitting file data ...
Committed revision 247084.
http://reviews.llvm.org/D12651
__
tfiala added a comment.
In http://reviews.llvm.org/D12651#241903, @zturner wrote:
> Cool, lgtm as well. Sorry for the holdup
Absolutely no worries. Thanks for checking, Zachary!
It would also be good if we could get either the --test-runner-name with
"threading" or "mulltiprocessing" workin
zturner added a comment.
Cool, lgtm as well. Sorry for the holdup
http://reviews.llvm.org/D12651
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
Cool, lgtm as well. Sorry for the holdup
On Tue, Sep 8, 2015 at 1:58 PM Adrian McCarthy wrote:
> amccarth added a comment.
>
> It seems to be general flakiness. In three runs without the patch, I got
> the lower numbers every time. In three runs with the patch, I got higher
> numbers twice an
amccarth added a comment.
It seems to be general flakiness. In three runs without the patch, I got the
lower numbers every time. In three runs with the patch, I got higher numbers
twice and the lower numbers once.
I have no objection to this patch based on this.
http://reviews.llvm.org/D126
tfiala added a comment.
In http://reviews.llvm.org/D12651#241810, @zturner wrote:
> Can you confirm with 2 runs before and 2 runs after the patch that you see
> the same results every time?
>
> Todd, can you think of a reason why this might happen?
Hmm, I vaguely recall finding what looked lik
amccarth added a comment.
I'm re-running the tests now, but I agree this is probably not worth holding up
this patch.
http://reviews.llvm.org/D12651
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/lis
Can you confirm with 2 runs before and 2 runs after the patch that you see
the same results every time?
Todd, can you think of a reason why this might happen? I don't think it's
worth holding the patch up too long over this because I want to see the
other improvements come through, but if you can
zturner added a comment.
Can you confirm with 2 runs before and 2 runs after the patch that you see
the same results every time?
Todd, can you think of a reason why this might happen? I don't think it's
worth holding the patch up too long over this because I want to see the
other improvements co
amccarth added a comment.
Ninja check-lldb works.
But...
Before the patch, there are 53 failing test suites and 71 failing test cases.
After the patch there are 54 failing test suites and 74 failing test cases.
Diffing the list of failed suites at the end of the output shows only one
differenc
zturner added a comment.
If the patch is busted, pretty much every test should start failing. As
long as ninja check-lldb actually runs, completes, and behaves pretty much
the same way as before, that's pretty much all that needs to be verified.
Ctrl+C shouldn't behave any differently with this
If the patch is busted, pretty much every test should start failing. As
long as ninja check-lldb actually runs, completes, and behaves pretty much
the same way as before, that's pretty much all that needs to be verified.
Ctrl+C shouldn't behave any differently with this patch, the idea was to
ref
amccarth added a subscriber: amccarth.
amccarth added a comment.
After applying the patch, I get three additional test case failures on Windows.
I'm trying to figure out now which ones.
Is there something specific I'm supposed to be trying, with regard to Ctrl+C
itself.
http://reviews.llvm.o
Adrian, can you verify this in the morning? Basically just trying to
ensure that ninja check-lldb still works as it did before. There's a
chance I'm going to be OOO tomorrow (or at the very best late) due to
something unexpected.
On Mon, Sep 7, 2015 at 9:38 PM Todd Fiala wrote:
> tfiala added
zturner added a comment.
Adrian, can you verify this in the morning? Basically just trying to
ensure that ninja check-lldb still works as it did before. There's a
chance I'm going to be OOO tomorrow (or at the very best late) due to
something unexpected.
http://reviews.llvm.org/D12651
_
tfiala added a comment.
@zturner, at this point you should be able to run this and see no change on
Windows (assuming I did the os check correctly). The Windows test runner is
set to be the previous multithreading-pool strategy. For everyone else,
they'll get the multithreading strategy by de
tfiala updated this revision to Diff 34186.
tfiala added a comment.
Adds "threading" test-runner strategy, which mirrors the "multithreading"
runner in terms of supporting Ctrl-C and hand-rolling the worker model. Like
multiprocessing over multiprocessing-pool, threading outperforms threading-p
tfiala updated this revision to Diff 34114.
tfiala added a comment.
This change adds the threading-module-based parallel test runner strategy. It
is not selected by default. It is pool-based.
The threading-pool and multiprocessing-pool strategies are 19.8% slower than
the newer hand-wrapped c
tfiala added a comment.
In http://reviews.llvm.org/D12651#240633, @zturner wrote:
> I won't be able to test this until Tuesday, but thanks for working on it
No worries. Have a great weekend!
(And maybe I'll have a working Windows setup before then).
http://reviews.llvm.org/D12651
___
I won't be able to test this until Tuesday, but thanks for working on it
On Sat, Sep 5, 2015 at 12:39 PM Todd Fiala wrote:
> tfiala added a comment.
>
> That last patch also fixes a regression in the previous patch when
> --threads 1 was specified --- the serial test runner in dosep.py was buste
zturner added a comment.
I won't be able to test this until Tuesday, but thanks for working on it
http://reviews.llvm.org/D12651
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
tfiala added a comment.
That last patch also fixes a regression in the previous patch when --threads 1
was specified --- the serial test runner in dosep.py was busted and I missed
that.
http://reviews.llvm.org/D12651
___
lldb-commits mailing list
tfiala added a subscriber: dawn.
tfiala updated this revision to Diff 34112.
tfiala added a comment.
The latest patch does the following:
- adds a test runner strategy layer for the isolated, concurrent test support
(in dosep.py).
The currently-supported test runners are:
1. multiprocessing
T
That'll also let me set it up so Greg can poke around with the threading
version on OS X.
On Fri, Sep 4, 2015 at 10:04 PM, Todd Fiala wrote:
> Yep, I'm thinking that's right.
>
> On Fri, Sep 4, 2015 at 10:02 PM, Zachary Turner
> wrote:
>
>> The pluggable method would at least allow everyone to
Yep, I'm thinking that's right.
On Fri, Sep 4, 2015 at 10:02 PM, Zachary Turner wrote:
> The pluggable method would at least allow everyone to continue working
> until someone has time to dig into what's wrong with multiprocess on Windows
>
> On Fri, Sep 4, 2015 at 9:56 PM Todd Fiala wrote:
>
>
The pluggable method would at least allow everyone to continue working
until someone has time to dig into what's wrong with multiprocess on Windows
On Fri, Sep 4, 2015 at 9:56 PM Todd Fiala wrote:
> On Fri, Sep 4, 2015 at 5:40 PM, Zachary Turner wrote:
>
>>
>>
>> On Fri, Sep 4, 2015 at 5:10 PM
On Fri, Sep 4, 2015 at 5:40 PM, Zachary Turner wrote:
>
>
> On Fri, Sep 4, 2015 at 5:10 PM Todd Fiala wrote:
>
>> tfiala added a comment.
>>
>> In http://reviews.llvm.org/D12651#240480, @zturner wrote:
>>
>> > Tried out this patch, unfortunately I'm seeing the same thing. The very
>> > first c
On Fri, Sep 4, 2015 at 5:10 PM Todd Fiala wrote:
> tfiala added a comment.
>
> In http://reviews.llvm.org/D12651#240480, @zturner wrote:
>
> > Tried out this patch, unfortunately I'm seeing the same thing. The very
> > first call to worker.join() is never returning.
> >
> > It's unfortunate tha
tfiala added a comment.
In http://reviews.llvm.org/D12651#240480, @zturner wrote:
> Tried out this patch, unfortunately I'm seeing the same thing. The very
> first call to worker.join() is never returning.
>
> It's unfortunate that it's so hard to debug this stuff, do you have any
> suggestion
Tried out this patch, unfortunately I'm seeing the same thing. The very
first call to worker.join() is never returning.
It's unfortunate that it's so hard to debug this stuff, do you have any
suggestions for how I can try to nail down what the child dotest instance
is actually doing? I wonder if
tfiala added a comment.
Hey Zachary,
Can you give the latest patch a try? That might stop some unintentional
blockage as adding items to those two queues would be blocking.
I'm going to be relocating home in a minute, but the other thing I'm going to
try is to go back to the multiprocessing.P
tfiala updated this revision to Diff 34089.
tfiala added a comment.
Specify tight queue sizes for the job description queue and the job results
queue.
This *might* stop unintentional queue blocking when adding items to the queue
if they were sized too small by default. Might be a possible diff
tfiala added a comment.
In http://reviews.llvm.org/D12651#240443, @zturner wrote:
> Correct, it doesn't do that prior to the patch. It looks like it's never
> exiting this loop:
>
> try:
> for worker in workers:
> worker.join()
> except:
>
>
> either when a Ctrl+C happens
Correct, it doesn't do that prior to the patch. It looks like it's never
exiting this loop:
try:
for worker in workers:
worker.join()
except:
either when a Ctrl+C happens, or when all the processes finish. I guess
it's stuck in there for some reason.
tfiala added a comment.
In http://reviews.llvm.org/D12651#240439, @zturner wrote:
> Hmm, yea the problem is worse than I thought. Even if I don't do Ctrl+C at
> all (I just run the test suite like I always have and let it finish) it's
> just leaving a bunch of stale processes at the end withou
clayborg added a comment.
Were you running dosep.py on Windows before as the way to run your test suite?
Or were you running dotest.py (no multi-threading)?
http://reviews.llvm.org/D12651
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
ht
clayborg added a comment.
I believe that python is also weird in that if you want to catch a CTRL+C it
has to be executing python code (not in a python code the entered C/C++ code)
otherwise the CTRL+C might not get to the python itself. This might be only a
problem in the embedded python inter
Hmm, yea the problem is worse than I thought. Even if I don't do Ctrl+C at
all (I just run the test suite like I always have and let it finish) it's
just leaving a bunch of stale processes at the end without them shutting
down on their own.
On Fri, Sep 4, 2015 at 3:39 PM Todd Fiala wrote:
> tfi
tfiala added a comment.
In http://reviews.llvm.org/D12651#240429, @zturner wrote:
> I can try to put a little time into figuring out what's wrong. I'd rather
> it not go in broken though, so let me take a look first and we can figure
> out what to do after that.
Yep absolutely.
http://revi
tfiala added a comment.
In http://reviews.llvm.org/D12651#240423, @zturner wrote:
> Ctrl+C once doesn't work on Windows with this patch. It seems to continue
> starting up new processes, and after all of them are done, I'm left with the
> original set of Python processes not doing any work, ju
I can try to put a little time into figuring out what's wrong. I'd rather
it not go in broken though, so let me take a look first and we can figure
out what to do after that.
On Fri, Sep 4, 2015 at 3:35 PM Greg Clayton wrote:
> clayborg accepted this revision.
> clayborg added a comment.
> This
clayborg accepted this revision.
clayborg added a comment.
This revision is now accepted and ready to land.
Looks fine. Zach, do you need to make some modifications for Windows? Or should
we just check this in and you can fix windows with a separate patch?
http://reviews.llvm.org/D12651
zturner added a comment.
Ctrl+C once doesn't work on Windows with this patch. It seems to continue
starting up new processes, and after all of them are done, I'm left with the
original set of Python processes not doing any work, just sitting there. If I
Ctrl+C again everything does.
Note tha
tfiala created this revision.
tfiala added reviewers: clayborg, zturner.
tfiala added a subscriber: lldb-commits.
This change does the following:
* If Ctrl-C is hit once, the parallel dotest.py runner will stop any further
work from being started, but will wait for the existing tests in progress
45 matches
Mail list logo
