Speed up AST dump by only including API headers #759
Conversation
| #include <workerd/io/compatibility-date.h> | ||
| #include <workerd/jsg/rtti.h> | ||
|
|
||
| #if !API_ENCODER_HDRS_ONLY |
There was a problem hiding this comment.
With such a large #if..#endif block, we could consider splitting this file into two files, something like "tools/workerd-apis.h" and "api-encoder.c++".
There was a problem hiding this comment.
I'm kind of torn on this but have a slight preference for keeping it as-is. Having a separate file for the headers would make the AST generation input more clean, but it would also mean having a file just containing 19 include directives that is of little use elsewhere (it could be used to include API headers elsewhere, but in my include cleanup patches I have been trying to avoid including the entire API when this is not strictly required; the size of the includes tends to negatively affect compile times). Overall, it makes sense to me referring to the api encoder source as it is also used for type generation and necessarily includes the entire API.
|
Nice improvement! It would be nice-to-have the comment that goes with the PR in the commit message too. |
Only compile the api headers when generating the clang ast dump. This reduces the size of the generated AST by about a quarter and allows us to reduce the time spent on AST generation, AST parsing and parameter extraction. This is particularly useful when compiling all targets such as on the CI build, where AST generation, parameter extraction and the subsequent type generation represent a bottleneck, so reducing the AST size should help improve the critical path compile time. The generated types are unaffected by this change.
fhanau
force-pushed
the
felix/ast-dump-opt
branch
from
5ee31a7 to
e04d8d6
Compare
Done! |
As part of a wider effort to improve CI compile times, only compile the api headers when generating the clang ast dump.
The reduced AST (which needs to be generated, gzip-compressed and parsed by param_extractor) is significantly smaller as
api.ast.json.gzgoes from 167MB to 119MB, further reducing CI disk usage. This improves the local runtime of generatingapi.ast.json.gzand subsequentlyparam-names.jsonfrom 27s to 20s. As this is a potential bottleneck when compiling all targets, it should help improve the critical path compile time.I have verified that the generated types (i.e.
//types:types) are identical.Note: upstream does not use either of the files affected, so no upstream PR is needed.