Page MenuHomePhabricator

Fix some issues with opt-viewer tests, plus disable them on Windows
ClosedPublic

Authored by zturner on Jan 5 2018, 12:58 PM.

Details

Summary

Back in D40202 I reported an issue that cpu_count() wasn't working properly. At the time I thought this was only for me since no bots were failing, but as it turns out this is happening across the board on Windows. The reason I'm the only one that caught it is because I'm the only who has all the necessary dependencies installed so that this test would run at all. So for everyone else, the test was just being treated as Unsupported.

I managed to fix many of the issues, but there is still a race condition that I've been unable to resolve. Specifically, I get this call stack:

Traceback (most recent call last):
  File "D:/src/llvm-mono/llvm/tools/opt-viewer/opt-viewer.py", line 339, in <module>
    main()
  File "D:/src/llvm-mono/llvm/tools/opt-viewer/opt-viewer.py", line 336, in main
    print_progress)
  File "D:/src/llvm-mono/llvm/tools/opt-viewer/opt-viewer.py", line 251, in generate_report
    should_print_progress)
  File "D:\src\llvm-mono\llvm\tools\opt-viewer\optpmap.py", line 50, in pmap
    result = pool.map(_wrapped_func, func_and_args, *args, **kwargs)
  File "C:\Python27\lib\multiprocessing\pool.py", line 251, in map
    return self.map_async(func, iterable, chunksize).get()
  File "C:\Python27\lib\multiprocessing\pool.py", line 567, in get
    raise self._value
AttributeError: type object 'Passed' has no attribute 'demangler_lock'

error: command failed with exit status: 1

I know it's a race condition because if I pass --jobs=1 then the tests pass. Since I don't know this code well, I'm not going to investigate this further, but I did want to at least try to get the rest of the issues I've found resolved.

The main source of issues comes from a fundamental difference in the way Windows and Linux handle multiprocessing in Python. If you have a global variable named args, and then you run a bunch of workers in a multiprocessing.Pool, on Windows args will be undefined but on non-Windows it will be defined. I managed to fix all of these by propagating the values through to the functions and classes that needed them, and I also fixed the cpu_count() issue by having the command line argument default to None if it's not specified. This way the multiprocessing module will figure this out internally (the documentation says that if processes=None, it will use the cpu count. This method works on Windows, even though (for some reason) calling cpu_count() directly doesn't.

To ensure that the global variable issue doesn't creep back in, I've wrapped the core logic of the script in a main() function and then call that function. This way args isn't a global variable, so if someone is relying on it being a global variable it will fail on non-Windows from now on as well.

Someone else will need to investigate this outstanding race condition. I don't believe it's specific to Windows, so in theory it should be reproducible anywhere.

Diff Detail

Repository
rL LLVM

Event Timeline

zturner created this revision.Jan 5 2018, 12:58 PM
rnk accepted this revision.Jan 5 2018, 1:09 PM

Python stuff looks good, but @anemet owns this so I'd ask him.

This revision is now accepted and ready to land.Jan 5 2018, 1:09 PM
anemet accepted this revision.Jan 5 2018, 1:42 PM

LGTM too. Thanks for getting back to this!

llvm/tools/opt-viewer/opt-stats.py
14 ↗(On Diff #128787)

Do we still need this after your change?

This revision was automatically updated to reflect the committed changes.