evmar / n2

n2 ("into"), a ninja compatible build system
Apache License 2.0
356 stars 26 forks source link

10% of parse time is tracking line numbers #107

Open evmar opened 9 months ago

evmar commented 9 months ago

We mark each build with its line number for use in error messages, but computing the line number requires keeping track of each newline we encounter.

Some experiments:

A trick I learned from LLVM is to instead annotate each build with its file byte offset when parsing, which is faster to gather. Then if we encounter an error we can spend the time to re-parse the file for newlines to find the offset. Parsing is slow but it's like milliseconds slow, it's fine to do in an error message codepath.

Unfortunately doing this would mean we need to either keep around the build file text at runtime, or re-read the file when generating an error message.

If we kept the file text at runtime:

If we re-read the file when generating error messages:

Colecf commented 9 months ago

I believe the Android build file text might be much larger(?)

Some numbers:

aosp-main$ du -h out/build-sdk_phone64_x86_64.ninja 
935M    out/build-sdk_phone64_x86_64.ninja
aosp-main$ du -h out/soong/build.sdk_phone64_x86_64.ninja
2.3G    out/soong/build.sdk_phone64_x86_64.ninja
internal-main$ du -h out/build-sdk_phone64_x86_64.ninja 
1.3G    out/build-sdk_phone64_x86_64.ninja
internal-main$ du -h out/soong/build.sdk_phone64_x86_64.ninja
3.9G    out/soong/build.sdk_phone64_x86_64.ninja

So yeah, it would be quite a bit of memory used just for line numbers...

evmar commented 9 months ago

I was about to say that's a convincing argument for not keeping the files in memory, but I guess if we mmap'd the files the memory use of keeping them around is "free", in that the memory backing of the mmap could be paged out by the OS under memory pressure... I think(?)

(I had thought that we only used line numbers in error messages, which would mean we'd only ever need to reread a file once. But looking now we also use them in -d explain mode to print which builds are out of date...)

Just to write it down, the full LLVM trick is: map each input file to an integer (e.g. put them in a Vec), and then pack that integer and the file offset together. So you only need to store a single integer per thing you want to annotate with a file location. This matters more for LLVM because I think they store the file location of every AST node (for compiler diagnostics etc.)

In n2's case we only store one of these per build statement. Currently it's a not especially efficient Rc+usize so 16 bytes per Build in memory. For a big Android build that's probably only still costing a few megabytes so probably not worth worrying about too much.

evmar commented 9 months ago

Note to self, to find all the places we use line numbers, the trick is to comment out the impl std::fmt::Display for FileLoc and see where the errors are.

Colecf commented 8 months ago

After https://github.com/evmar/n2/pull/112, I tried disabling the line counting and wasn't able to observe a noticeable speedup.