> On Sept. 15, 2016, 3:16 a.m., Kevin Klues wrote: > > support/mesos-gtest-runner.py, lines 36-37 > > <https://reviews.apache.org/r/51715/diff/1/?file=1494165#file1494165line36> > > > > What is a shard in this context?
I introduce this term in the script docstring now. > On Sept. 15, 2016, 3:16 a.m., Kevin Klues wrote: > > support/mesos-gtest-runner.py, line 63 > > <https://reviews.apache.org/r/51715/diff/1/?file=1494165#file1494165line63> > > > > I would prefer to see this function inline under the `__main__` > > directive below, and instead have a helper function for parsing the args > > instead of inlining that content as done below. My guideline here was to have functionality accessing global scope at global scope, and isolated pieces in functions. I understand that your suggestion is more along the lines of the general Python style, so I aligned the code with that now. > On Sept. 15, 2016, 3:16 a.m., Kevin Klues wrote: > > support/mesos-gtest-runner.py, line 70 > > <https://reviews.apache.org/r/51715/diff/1/?file=1494165#file1494165line70> > > > > At first glance it's not clear to me what this function is used for. > > Can you add a more descriptive comment about what this is used for? > > > > i.e. explain how this is passed to each subcommand that gets spawned > > asynchronously in order to pass it the proper options I extended the docstring a bit, please let me know if you believe it still needs further explanation. > On Sept. 15, 2016, 3:16 a.m., Kevin Klues wrote: > > support/mesos-gtest-runner.py, line 154 > > <https://reviews.apache.org/r/51715/diff/1/?file=1494165#file1494165line154> > > > > How did you come up with this default number of jobs? I would think we > > should spawn slightly *less* jobs than the number of CPUs on the machine > > (not 1.5 times more). > > > > Also it's not a big deal since we only have one constant for now, but I > > would prefer to move this line to the top of the file instead of in line > > here (e.g. similar to how we keep all constants at the top of the file in > > support/generate-endpoint-help.py). If we keep it here, I would add a > > newline after it. Very rarely will our jobs make full use of a single let alone multiple CPUs. In fact it seems that a large fraction of the test execution time is spent waiting for messages to arrive. I found that slightly oversubscribing the machine let to better utilization, at least on my machine with 8 virtual cores. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51715/#review149005 ----------------------------------------------------------- On Sept. 19, 2016, 4:35 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51715/ > ----------------------------------------------------------- > > (Updated Sept. 19, 2016, 4:35 p.m.) > > > Review request for mesos, Kevin Klues and Till Toenshoff. > > > Bugs: MESOS-6140 > https://issues.apache.org/jira/browse/MESOS-6140 > > > Repository: mesos > > > Description > ------- > > This adds `b814987b28cfb65a7c9635c83399545e423e690a` of > https://github.com/bbannier/gtest-shrdr. > > > Diffs > ----- > > support/mesos-gtest-runner.py PRE-CREATION > > Diff: https://reviews.apache.org/r/51715/diff/ > > > Testing > ------- > > Tested with e.g., `stout-tests` > > $ ./support/mesos-gtest-runner.py build/3rdparty/stout/stout-tests > ............ > [PASS] > > > Thanks, > > Benjamin Bannier > >
