GN should only load each import once |
|||
Issue descriptionIn GN mutliple BUILD files can load the same import. GN caches the results of imports so we don't have to load them more than once. But if two BUILD files load the same import at the same time, there is a race. Rather than lock, the code allows each to load the file and the first one finished "wins". This is based on the theory that the race is rare and processing imports is relatively fast. On Windows, many build files end up with the visual_studio_version.gni file which ends up calling build/vs_toolchain.py. This script can be quite slow (slower than the rest of the entire GN run in some cases). The result is that the race is guaranteed to happen for basically every BUILD file that references the .gni file, and we end up running the script many times in parallel (which only slows it down more). We should add the extra locking to resolve the race before loading rather than after.
,
May 6 2016
,
May 6 2016
,
May 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a5b852cfbe6b6912ee6d1af145cbe9c090e40b42 commit a5b852cfbe6b6912ee6d1af145cbe9c090e40b42 Author: brettw <brettw@chromium.org> Date: Mon May 09 17:40:46 2016 GN: Don't import a file more than once. GN caches the results of imports so we don't have to load them more than once. But if two BUILD files load the same import at the same time, there is a race Previously GN would resolve the winner after the import ran on the assumption that imports are fast and collisions are rare. But some imports run expensive scripts meaning they take a long time and collisions are likely. The result is that a file can get imported more than once. This patch adds extra locking around the import to block other threads and only do the import once. Increase the minimum thread count for low-end systems to 8. This gives the fastest running time on an older MacBook. Both the import change and the thread change independently speed up GN on Mac Chrome by 150-300ms, so actual improvement should be 300-600ms. On beefy Windows and Linux workstations, there seems to be no change in performance with these changes, although the import change should help some worst-case situations. BUG= 599892 Review-Url: https://codereview.chromium.org/1957483004 Cr-Commit-Position: refs/heads/master@{#392353} [modify] https://crrev.com/a5b852cfbe6b6912ee6d1af145cbe9c090e40b42/tools/gn/import_manager.cc [modify] https://crrev.com/a5b852cfbe6b6912ee6d1af145cbe9c090e40b42/tools/gn/import_manager.h [modify] https://crrev.com/a5b852cfbe6b6912ee6d1af145cbe9c090e40b42/tools/gn/scheduler.cc
,
May 9 2016
|
|||
►
Sign in to add a comment |
|||
Comment 1 by brettw@chromium.org
, May 5 2016