This is an archive of the discontinued LLVM Phabricator instance.

[Darwin] Limit parallelism for sanitizer tests that use shadow memory on AS
ClosedPublic

Authored by yln on Mar 30 2022, 10:55 AM.

Details

Summary

On Darwin, we want to limit the parallelism during test execution for
sanitizer tests that use shadow memory. The reason is explained by this
existing comment:

Only run up to 3 processes that require shadow memory simultaneously
on 64-bit Darwin. Using more scales badly and hogs the system due to
inefficient handling of large mmap'd regions (terabytes) by the
kernel.

Previously we detected 3 cases:

  • on-device: limit to 1 process
  • 64-bit: macOS & simulators, limit to 3 processes
  • others (32-bit): no limitation

We checked for the 64-bit case like this: `if arch in ['x86_64',
'x86_64h']` which misses macOS running on AS. Additionally, we don't
care about 32-bit anymore, so I've simplified this to 2 cases: on-device
and everything else.

Diff Detail

Event Timeline

yln created this revision.Mar 30 2022, 10:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 30 2022, 10:55 AM
Herald added a subscriber: pengfei. · View Herald Transcript
yln requested review of this revision.Mar 30 2022, 10:55 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 30 2022, 10:55 AM
Herald added a subscriber: Restricted Project. · View Herald Transcript

Changes seems good. I certainly like that we've simplified things. If we can resolve the question I asked then I think this will be good to go.

compiler-rt/test/asan/Unit/lit.site.cfg.py.in
65

How do we ensure this change doesn't effect non-Darwin hosts?

Previously we would only assign a parallelism group on Darwin hosts, now we don't have that check anymore.

Is it okay to do this because assigning a parallelism group on its own won't change anything. Presumably it also requires the lit_config.parallelism_groups['shadow-memory'] = 3 setting which is guarded by platform.system() == 'Darwin'?

yln marked an inline comment as done.Mar 31 2022, 2:34 PM
yln added inline comments.
compiler-rt/test/asan/Unit/lit.site.cfg.py.in
65

Yes, that's right. There is two steps:

  • Assigning a parallelism group to test configs, that is, declare that "tests for this tool setup shadow memory".
  • Per platform configuration of the constraints:
# No throttling on non-Darwin platforms.
lit_config.parallelism_groups['shadow-memory'] = None

if platform.system() == 'Darwin':
  ios_device = config.apple_platform != 'osx' and not config.apple_platform.endswith('sim')
  # Force sequential execution when running tests on iOS devices.
  if ios_device:
    lit_config.warning('Forcing sequential execution for iOS device tests')
    lit_config.parallelism_groups['ios-device'] = 1
    config.parallelism_group = 'ios-device'

  # Only run up to 3 processes that require shadow memory simultaneously on
  # 64-bit Darwin. Using more scales badly and hogs the system due to
  # inefficient handling of large mmap'd regions (terabytes) by the kernel.
  else:
    lit_config.warning('Throttling sanitizer tests that require shadow memory on Darwin')
    lit_config.parallelism_groups['shadow-memory'] = 3

This is already the standard to configure the first part:

if not config.parallelism_group:
  config.parallelism_group = 'shadow-memory'

Except for unit tests (which don't have an explicit arch and just compile for the host), so we had a special callback to derive the arch from the test name. Since we don't need to distinguish between 32/64-bit anymore, we can just remove this part.

delcypher accepted this revision.Mar 31 2022, 2:37 PM

LGTM

compiler-rt/test/asan/Unit/lit.site.cfg.py.in
65

Thanks for explaining.

This revision is now accepted and ready to land.Mar 31 2022, 2:37 PM
This revision was automatically updated to reflect the committed changes.
yln marked an inline comment as done.
compiler-rt/unittests/lit.common.unit.cfg.py