gn needlessly writes stamp files for actions with one output |
||||
Issue descriptionSee e.g. this: build gen/blink/platform/ColorData.cpp: __third_party_WebKit_Source_platform_color_data___build_toolchain_linux_clang_x64__rule | obj/third_party/WebKit/Source/platform/color_data.inputdeps.stamp build obj/third_party/WebKit/Source/platform/color_data.stamp: stamp gen/blink/platform/ColorData.cpp Having a stamp rule for this is 100% waste, things depending on color_data.stap should just dep on ColorData directly. Also, the inputdeps could be inlined: build obj/third_party/WebKit/Source/platform/color_data.inputdeps.stamp: stamp ../../third_party/WebKit/Source/build/scripts/gperf.py ../../third_party/WebKit/Source/platform/ColorData.gperf For stamp files that are used only once, this is always a win since the ninja file gets smaller and we don't need the stamp files. I'd guess that we could get rid of maybe a little less than 3k stamp edges in the chrome build if we manage to talk gn into not writing these two kinds of stamp files for actions. I'll see if I can cook up a patch. The latter half is handled at https://cs.chromium.org/chromium/src/tools/gn/ninja_action_target_writer.cc?q=stamp+file:tools/gn&sq=package:chromium&dr=C&l=41 and it even has a comment pointing this out. The former half is here I think: https://cs.chromium.org/chromium/src/tools/gn/ninja_action_target_writer.cc?q=stamp+file:tools/gn&sq=package:chromium&dr=C&l=94
,
Feb 10 2018
This is a proof of concept for inlining action input deps: https://chromium-review.googlesource.com/#/c/chromium/src/+/912536 This reduces the number of stamp edges by 1817 (and the number of lines in toolchain.ninja by 1817, and that file's size by ~3MB).
,
Feb 12 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cf54d20bfce0f557b4d6d5cea5d46ce1c2fc7909 commit cf54d20bfce0f557b4d6d5cea5d46ce1c2fc7909 Author: Nico Weber <thakis@chromium.org> Date: Mon Feb 12 18:19:41 2018 gn: Write no stamp files for action inputs. Also omit input stamp files that are referenced only once in other places. Stamp files are apparently somewhat expensive to create on Windows (crbug.com/787903) Removes 1817 stamp files from the main toolchain.ninja file in my chrome/linux build, and reduces toolchain.ninja size by 3MB. No intended behavior change. Bug: 810978 Change-Id: I33ed07bc473b245e4c27414012681f238980e9fa Reviewed-on: https://chromium-review.googlesource.com/912536 Reviewed-by: Brett Wilson <brettw@chromium.org> Commit-Queue: Nico Weber <thakis@chromium.org> Cr-Commit-Position: refs/heads/master@{#536132} [modify] https://crrev.com/cf54d20bfce0f557b4d6d5cea5d46ce1c2fc7909/tools/gn/ninja_action_target_writer.cc [modify] https://crrev.com/cf54d20bfce0f557b4d6d5cea5d46ce1c2fc7909/tools/gn/ninja_action_target_writer.h [modify] https://crrev.com/cf54d20bfce0f557b4d6d5cea5d46ce1c2fc7909/tools/gn/ninja_action_target_writer_unittest.cc [modify] https://crrev.com/cf54d20bfce0f557b4d6d5cea5d46ce1c2fc7909/tools/gn/ninja_binary_target_writer.cc [modify] https://crrev.com/cf54d20bfce0f557b4d6d5cea5d46ce1c2fc7909/tools/gn/ninja_binary_target_writer.h [modify] https://crrev.com/cf54d20bfce0f557b4d6d5cea5d46ce1c2fc7909/tools/gn/ninja_bundle_data_target_writer.cc [modify] https://crrev.com/cf54d20bfce0f557b4d6d5cea5d46ce1c2fc7909/tools/gn/ninja_copy_target_writer.cc [modify] https://crrev.com/cf54d20bfce0f557b4d6d5cea5d46ce1c2fc7909/tools/gn/ninja_target_writer.cc [modify] https://crrev.com/cf54d20bfce0f557b4d6d5cea5d46ce1c2fc7909/tools/gn/ninja_target_writer.h [modify] https://crrev.com/cf54d20bfce0f557b4d6d5cea5d46ce1c2fc7909/tools/gn/ninja_target_writer_unittest.cc
,
Feb 12 2018
Latter half (inputdeps) done, former half (output stamp) still to go. And needs a gn roll to pick up the change in comment 3.
,
Feb 12 2018
There's a couple of changes to be rolled in at this point, so I'm happy to do another roll. Do you want me to do one now, or wait for a "former half" change?
,
Feb 12 2018
No need to wait, I haven't even started on the other half and I'm not sure when I'll get around to it. (Hopefully some time this week, but who knows.)
,
Feb 12 2018
ok.
,
Apr 26 2018
,
Aug 3
This bug has an owner, thus, it's been triaged. Changing status to "assigned". |
||||
►
Sign in to add a comment |
||||
Comment 1 by brucedaw...@chromium.org
, Feb 10 2018