use parallel-hashmap

[atetubou]

This makes noop build time around 12% faster in chromium.

TODO: get benchmark resulsts from hyperfine

[digit-google]

Interesting, can you make this a build configuration option instead?

Since this is a header-only library, just passing the path to its include directory should be sufficient. Either as a CMAKE custom variable (e.g. NINJA_PHASHMAP_INCLUDE_DIR) or a configure.py option such as --phashmap-include-dir. Both of which could set a macro used in src/hash_map.h to select the implementation.

[jhasse]

How does this compare against std::flat_map?

I would like to avoid adding a Git submodule since it breaks tarball downloads and is annoying in general. I think a FetchContent download would be better.

I also wouldn't like a build configuration option, because it creates another code variant to test.

[digit-google]

Point taken. I would still welcome a configuration option that allows us to pass the path of the phashmap include directory, and thus avoid a network download, as this will greatly simplify our CI builder configurations (just like with do for GoogleTest currently :-)). How do you plan to implement FetchContent in configure.py though?

Regarding std::flat_map, this is implemented as a linear vector of (key,value) pairs, and has O(N) insertion costs. Its typically good when the number of items is small (e.g. < 1000) or when all insertions are performed initially (e.g. pass an unordered vector to the constructor, which just performs an in-place sort).

For Ninja, we have build plans with 900,000+ nodes in them, and the map are mutated in several places that can be distant in time (e.g. when dynamic dependencies are injected in it), so I don't think this type is a good fit here. Plus we don't need the ordering guarantee and the associated cost.

[atetubou]

Hmm, for source code tarball we may do something like this with another tarball containing submodule too.

We can still use FetchContent and have similar functionality in configure.py, but showing caution in release page might be simpler?

[digit-google]

I suggest keeping things simple here:

1. If Git submodules are not appropriate, just copy the phashmap sources from a known revision under src/phashmap/ or third_party/phashmap (your choice) with the appropriate LICENSE file, and a README.md indicating the upstream URL and gut commit used to populate it. This is after all equivalent to a git checkout with one submodule (that doesn't have its own dependencies).

2. If you don't want to do that, add an option to pass the include dir of the library at configure time for both CMake and configure.py, because I guarantee you that FetchContent / whatever download-at-configure-time solution is going to break some workflows somewhere if you don't provide an escape hatch for it.