Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use GN instead of Ninja to generate compile commands and save ~1sec off of no-op builds. #25737

Merged
merged 2 commits into from
Apr 29, 2021

Conversation

chinmaygarde
Copy link
Member

It used to be that GN could not generate compile commands on its own. Instead,
we would use Ninja to do the same in a separate invocation after GN. This worked
but the invocation took about a second to complete. Now that GN can generate the
compile commands as it is generating the Ninja build rules, we ask it generate
the compile commands directly. The commands seem identical and IDE integrations
continue to work.

One difference now is that the compile commands are now generated in the build
directory instead of the out directory like earlier. I think this is easier to
reason about but if others think we need the old behavior, I can add a symlink
to the compile_commands.json.

This knocks 0.9 to 1 seconds off of a no-op build compared to numbers in
flutter/flutter#81074.

$ time ./flutter/tools/gn --unopt
Generating GN files in: out/host_debug_unopt
Generating Xcode projects took 244ms
Generating compile_commands took 104ms
Done. Made 739 targets from 228 files in 1781ms
./flutter/tools/gn --unopt  3.07s user 3.04s system 325% cpu 1.875 total

Related to flutter/flutter#81074

It used to be that GN could not generate compile commands on its own. Instead,
we would use Ninja to do the same in a separate invocation after GN. This worked
but the invocation took about a second to complete. Now that GN can generate the
compile commands as it is generating the Ninja build rules, we ask it generate
the compile commands directly. The commands seem identical and IDE integrations
continue to work.

One difference now is that the compile commands are now generated in the build
directory instead of the out directory like earlier. I think this is easier to
reason about but if others think we need the old behavior, I can add a symlink
to the compile_commands.json.

This knocks 0.9 to 1 seconds off of a no-op build compared to numbers in
flutter/flutter#81074.

```
$ time ./flutter/tools/gn --unopt
Generating GN files in: out/host_debug_unopt
Generating Xcode projects took 244ms
Generating compile_commands took 104ms
Done. Made 739 targets from 228 files in 1781ms
./flutter/tools/gn --unopt  3.07s user 3.04s system 325% cpu 1.875 total

```

Related to flutter/flutter#81074
@chinmaygarde chinmaygarde added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Apr 23, 2021
Copy link
Contributor

@dnfield dnfield left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, LGTM

Might have to figure out what to do with IDE settings for this, but it'll be better I think because the IDE won't suddenly be changing just because of what your last GN command was.

@chinmaygarde
Copy link
Member Author

Yeah, we can make the IDE stuff opt-in. But the 244 ms it takes is well away from being the bottleneck for now. Besides, that is generated in the single GN invocation.

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite build_and_test_linux_unopt_debug has failed. Please fix the issues identified (or deflake) before re-applying this label.

@fluttergithubbot fluttergithubbot removed the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Apr 23, 2021
@chinmaygarde chinmaygarde added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Apr 29, 2021
@chinmaygarde
Copy link
Member Author

I've filed a flake report and re-run the test.

@fluttergithubbot fluttergithubbot merged commit 3430bf9 into flutter:master Apr 29, 2021
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Apr 30, 2021
gspencergoog pushed a commit to gspencergoog/engine that referenced this pull request May 5, 2021
@chinmaygarde chinmaygarde deleted the compile_commands_update branch July 7, 2022 22:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.
Projects
None yet
4 participants