Add automatic detection of the number of CPU core to trial -j

[therve]

It'd be nice if trial -j auto twisted detected the number of available cores and used that as the number of local workers.

[itamar]

The Python standard library multiprocessing module has a function that exposes the number of available CPUs.

[racheesingh]

I looked at the Python docs for multiprocessing. multiprocessing.cpu_count() seems like something we could use to get the number of CPUs. But the  docs also mention this:

Return the number of CPUs in the system. May raise NotImplementedError.

Apparently, on Windows machines cpu_count may result in a NotImplementedError. So should we use this at all?

[racheesingh]

Does this seem like a reasonable thing to do? [I tried out multiprocessing.cpu_count and modified the opt_jobs method like so.]

    def opt_jobs(self, number):
        """
        Number of local workers to run, a strictly positive integer.
        """
        # Case insensitive check if jobs was "auto"
        if number.lower() == "auto":
            try:
                number = multiprocessing.cpu_count()
            except NotImplementedError:
                # Windows might raise NotImplementedError while accessing cpu_count
                raise usage.UsageError(
                    "-j AUTO option not supported for your system.")
        else:
            try:
                number = int(number)
            except ValueError:
                raise usage.UsageError(
                    "Expecting integer argument to jobs, got '%s'" % number)
            if number <= 0:
                raise usage.UsageError(
                    "Argument to jobs must be a strictly positive integer")
        self["jobs"] = number

[racheesingh]

Patch file for the proposed fix for Ticket #5824

[Julian]

Hey!

Thanks for working on this. The solution you submitted looks like it's headed in the right direction, but all patches submitted to Twisted have to have accompanying tests. Take a gander at http://twistedmatrix.com/trac/wiki/TwistedDevelopment if you haven't already. You'll need to test the new behavior that this ticket is proposing.

Thanks again for looking at this!

[racheesingh]

Thanks for the pointers! I followed the links about how to add tests and I added a test in what seemed like the place where tests for trial located. After adding the test case I ran

 trial --testmodule twisted/scripts/trial.py

to check if my test case was running at all. But I found that it was not being run. Is there something I am missing here? I am attaching the patch file which has the test case I was trying to add.

[racheesingh]

Proposed fix along with test case.

[thijs]

Thanks for your patch.

1. you should run trial twisted.trial.test.test_script since you placed your test there
2. the docstring should also mention it can raise a NotImplementedError
3. or perhaps it should simply return 1 for platforms like win32 and print a warning instead?

[itamar]

Thanks for that patch!

I'm not sure what thijs is referring to in items 2 and 3. Here are my review comments:

1. "trial --help" should indicate auto is a possible option for --jobs, otherwise no one will know about the cool new feature you added!
2. You should provide a test for the error case where NotImplementedError is raised.
3. "-j AUTO option not supported for your system." would better read as "'-j auto option not supported for your system."`
4. You should provide a news file ( https://twistedmatrix.com/trac/wiki/ReviewProcess#Newsfiles) - "svn add" it and then "svn diff" will include it.

Please resubmit after addressing these issues.

[racheesingh]

Thanks for the review comments. I have incorporated the comments from thijs and itamar and am attaching the diff. Please do tell me if I have missed something out (or misunderstood your feedback).

[racheesingh]

Proposed fix along with test case, incorporating review comments.