Cache Cargo.toml read result if successful #20
Conversation
proc-macros should be deterministic, and although it shouldn't matter in this specific case, using HashMaps is an easy way to introduce non-determinism (through iteration order). toml's Map type is backed by either BTreeMap or IndexMap depending on the activated features, both of which are fully deterministic.
src/lib.rs
Outdated
| struct Cache { | ||
| manifest_dir: String, | ||
| manifest_path: PathBuf, | ||
| manifest: Table, |
There was a problem hiding this comment.
Before we merge this, could you try to directly cache all crate names. So, have a hash table here that maps from crate name to crate name in the cargo file? By all I mean all crates that are referenced in the cargo.toml. May also brings some improvements?
There was a problem hiding this comment.
I implemented this. I think there is actually a behavioral change though: When you have multiple dependencies on the same package, previously the first one in the order they were searched was used to determine the crate name, now it's the last one in the order they are iterated to build the package > crate mapping (because, AFAIK, when collect()ing a BTreeMap the last occurrence of any given key "wins").
I'm pretty sure those two orderings are the same the way I implemented it now, so when a package is depended on with different crate names, a different one will be returned from crate_name now. I think assume this acceptable, just wanted to spell it out.
There was a problem hiding this comment.
Yeah that sounds right, but this is also some very niche use case that wasn't tested before.
There was a problem hiding this comment.
BTW, did that improve the performance?
There was a problem hiding this comment.
I can benchmark again next week.
There was a problem hiding this comment.
This change doesn't seem to have a measureable impact on performance.
Closes #19.
Within ruma-client-api, this reduces compile time of
cargo clean && cargo check --features clientfrom around 20.5s to around 19s on my machine. Out of that time, around 8.5s are for dependencies that don't use proc-macro-crate, so the real improvement is probably closer to 12s => 10.5s (12.5%).