pnpm / pacquet

experimental package manager for node.js
Apache License 2.0
762 stars 21 forks source link

refactor: use utf8 strings as mem cache keys #222

Closed KSXGitHub closed 9 months ago

KSXGitHub commented 9 months ago

Initially, the use of OsString was to allow non-UTF8 paths. However, the index file feature requires serde which in turns require UTF-8 to serialize. So the need for OsString ceases to be necessary, and removing it open new opportunities for some micro-optimizations.

codecov[bot] commented 9 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (6d8cec2) 92.68% compared to head (c2d89d3) 92.68%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #222 +/- ## ======================================= Coverage 92.68% 92.68% ======================================= Files 57 57 Lines 2871 2872 +1 ======================================= + Hits 2661 2662 +1 Misses 210 210 ```

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

github-actions[bot] commented 9 months ago

Micro-Benchmark Results

Linux

group                          main                                   pr
-----                          ----                                   --
tarball/download_dependency    1.01      6.7±0.28ms   650.0 KB/sec    1.00      6.6±0.21ms   654.7 KB/sec
github-actions[bot] commented 9 months ago

Integrated-Benchmark Report (Linux)

Scenario: Frozen Lockfile

Command Mean [ms] Min [ms] Max [ms] Relative
pacquet@HEAD 127.2 ± 9.8 114.5 144.3 1.03 ± 0.11
pacquet@main 123.2 ± 9.8 107.5 137.7 1.00
BENCHMARK_REPORT.json ```json { "results": [ { "command": "pacquet@HEAD", "mean": 0.1271716063466667, "stddev": 0.009812358487650672, "median": 0.12619836648, "user": 0.060102726666666675, "system": 0.2001150733333333, "min": 0.11450732448000002, "max": 0.14428669348, "times": [ 0.14428669348, 0.11990778348000002, 0.12619836648, 0.13605476948, 0.12718388648, 0.11450732448000002, 0.12066137548000001, 0.13758830448, 0.12542734648, 0.12743158148, 0.13692406248, 0.11953791848000002, 0.14075418748000001, 0.11592374748000002, 0.11518674748000002 ], "exit_codes": [ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 ] }, { "command": "pacquet@main", "mean": 0.12316214754666668, "stddev": 0.009758218348705645, "median": 0.12175780848000001, "user": 0.05872752666666667, "system": 0.20377233999999997, "min": 0.10750951548000001, "max": 0.13768289148, "times": [ 0.13554187048, 0.11585921048000002, 0.13768289148, 0.11726989848000001, 0.11095596748000001, 0.11996457548000002, 0.12027272148000001, 0.13042775948, 0.11030591948000001, 0.13455231048, 0.12728008348, 0.12497080248000002, 0.12175780848000001, 0.13308087848, 0.10750951548000001 ], "exit_codes": [ 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 ] } ] } ```
zkochan commented 9 months ago

That's fine. Neither pnpm nor the other node.js based alternatives support non-utf8 chars.