pacquet icon indicating copy to clipboard operation
pacquet copied to clipboard

feat: add http caching to reqwest

Open anonrig opened this issue 2 years ago • 8 comments

before

Benchmark 1: pacquet
  Time (mean ± σ):     799.0 ms ±  17.2 ms    [User: 141.8 ms, System: 818.4 ms]
  Range (min … max):   777.5 ms … 827.8 ms    10 runs

after

Benchmark 1: pacquet
  Time (mean ± σ):     656.5 ms ±  26.4 ms    [User: 152.1 ms, System: 811.8 ms]
  Range (min … max):   625.6 ms … 709.3 ms    10 runs

anonrig avatar Aug 11 '23 15:08 anonrig

Anyway, I will review this later. Maybe tomorrow, or maybe next week.

KSXGitHub avatar Aug 11 '23 15:08 KSXGitHub

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (40f1291) 85.39% compared to head (406dfc8) 85.39%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #75   +/-   ##
=======================================
  Coverage   85.39%   85.39%           
=======================================
  Files          24       24           
  Lines        1239     1253   +14     
=======================================
+ Hits         1058     1070   +12     
- Misses        181      183    +2     
Files Changed Coverage Δ
crates/registry/src/lib.rs 0.00% <ø> (ø)
crates/cli/src/package.rs 100.00% <100.00%> (ø)
crates/cli/src/package_manager.rs 93.33% <100.00%> (+7.61%) :arrow_up:
crates/registry/src/package.rs 88.15% <100.00%> (-1.32%) :arrow_down:
crates/registry/src/package_version.rs 91.11% <100.00%> (-2.23%) :arrow_down:

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Aug 11 '23 15:08 codecov[bot]

Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.05     10.2±0.60ms   426.6 KB/sec    1.00      9.7±0.53ms   447.2 KB/sec

github-actions[bot] avatar Aug 11 '23 15:08 github-actions[bot]

I will make a prediction: This will likely be rendered irrelevant once a proper hashmap-based caching mechanism is implemented.

KSXGitHub avatar Aug 11 '23 15:08 KSXGitHub

You should fix the cargo deny.

I think we shouldn't follow this pattern, and start working on the hashmap-based caching mechanism.

anonrig avatar Aug 14 '23 18:08 anonrig

@anonrig Since we have merged hashmap based caching mechanism and full parallelization, you can now do a benchmark (in pacquet install) to see if this PR could bring performance improvement.

KSXGitHub avatar Aug 24 '23 09:08 KSXGitHub

It looks like we've just added mem-cache to the tarball download, should we add a cache for fetching the package from the registry as well?

Whether we implement mem-cache or not, perhaps http_caching stored on disk still has some value?

await-ovo avatar Nov 24 '23 03:11 await-ovo

@await-ovo Currently, all network requests in pacquet is performed via a single wrapper: pacquet-network. Cache may be added there.

KSXGitHub avatar Nov 24 '23 06:11 KSXGitHub