search

rust-secure-code / cargo-supply-chain

Gather author, contributor and publisher data on crates in your dependency graph.
Apache License 2.0
313 stars 18 forks source link

Less cloning #42

Closed BlackHoleFox closed 3 years ago

BlackHoleFox commented 3 years ago

These are just some allocation saving and cleanup changes I noticed while working on #41.

Feel free to just close this if you don't want the changes.

Shnatsel commented 3 years ago

Keeping String around was a deliberate decision to keep the codebase simple and stupid. I want it to be as accessible to external contributors as possible.

Does this measurably improve performance or memory use on any workloads? If not, I'd prefer to stick to cloning.

Shnatsel commented 3 years ago

The part that loads the list of crates barely even registers on the profile:

https://profiler.firefox.com/public/p9y0361gtbabmjvtrd1mje18kjwqjekx91zbpv0/flame-graph/?globalTrackOrder=0-1&localTrackOrderByPid=4674-0~4675-0~&thread=0&timelineType=stack&v=5

Resolving it within the cargo metadata process takes 100ms, but I can't see any noticeable time spent parsing it in cargo supply-chain process. For cached data that just takes 3 seconds to deserialize some JSON, and most of that in turn is spent doing I/O. I wonder if we forgot buffering somewhere...

Shnatsel commented 3 years ago

Okay, yeah, we did forgot to buffer file I/O. Fixed in commit 2b016eae9e2b68f5ba56af7a19d16cc6ff1982c8

Shnatsel commented 3 years ago

Even with buffering, this part of the code takes a negligible amount of time, so I don't think it's worth optimizing.

Here's a profile from running it on this very repo:

https://profiler.firefox.com/public/0eg11c6ez14bfm6crr5e1r8hjadj861cnc7z57g/flame-graph/?globalTrackOrder=0-1&localTrackOrderByPid=9404-0~9408-0~&thread=0&timelineType=stack&v=5

I'm closing this PR, but it did result in a 10x speedup, so thanks!

BlackHoleFox commented 3 years ago

I didn't expect it to give a noticeable speedup, but I'm glad it did result in the buffering giving a real one :D