Skip to content

perf(tspath): avoid string copy in ToFileNameLowerCase #1575

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

camc314
Copy link
Contributor

@camc314 camc314 commented Aug 13, 2025

This PR delivers ~10% perf improvement to ToFileNameLowerCase, as well as halving the number of allocations in the worst case.

~/go/bin/benchstat old.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/microsoft/typescript-go/internal/tspath
cpu: Apple M2 Max
                                                     │   old.txt    │               new.txt               │
                                                     │    sec/op    │    sec/op     vs base               │
ToFileNameLowerCase//path/to/file.ext-12               8.440n ± ∞ ¹   8.461n ± ∞ ¹        ~ (p=1.000 n=5)
ToFileNameLowerCase//PATH/TO/FILE.EXT-12               38.02n ± ∞ ¹   28.26n ± ∞ ¹  -25.67% (p=0.008 n=5)
ToFileNameLowerCase//path/to/FILE.EXT-12               41.81n ± ∞ ¹   30.36n ± ∞ ¹  -27.39% (p=0.008 n=5)
ToFileNameLowerCase//user/UserName/proje...etc-12      58.12n ± ∞ ¹   47.17n ± ∞ ¹  -18.84% (p=0.008 n=5)
ToFileNameLowerCase//user/UserName/proje...etc#01-12   162.0n ± ∞ ¹   160.3n ± ∞ ¹   -1.05% (p=0.008 n=5)
ToFileNameLowerCase//user/UserName/proje...etc#02-12   155.3n ± ∞ ¹   155.0n ± ∞ ¹        ~ (p=0.690 n=5)
ToFileNameLowerCase//user/UserName/proje...etc#03-12   135.8n ± ∞ ¹   136.5n ± ∞ ¹        ~ (p=1.000 n=5)
ToFileNameLowerCase/FoO/FoO/FoO/FoO/FoO/...etc-12      541.7n ± ∞ ¹   507.0n ± ∞ ¹   -6.41% (p=0.008 n=5)
geomean                                                78.51n         70.19n        -10.60%
¹ need >= 6 samples for confidence interval at level 0.95

                                                     │   old.txt   │               new.txt                │
                                                     │    B/op     │    B/op      vs base                 │
ToFileNameLowerCase//path/to/file.ext-12               0.000 ± ∞ ¹   0.000 ± ∞ ¹        ~ (p=1.000 n=5) ²
ToFileNameLowerCase//PATH/TO/FILE.EXT-12               48.00 ± ∞ ¹   24.00 ± ∞ ¹  -50.00% (p=0.008 n=5)
ToFileNameLowerCase//path/to/FILE.EXT-12               48.00 ± ∞ ¹   24.00 ± ∞ ¹  -50.00% (p=0.008 n=5)
ToFileNameLowerCase//user/UserName/proje...etc-12      96.00 ± ∞ ¹   48.00 ± ∞ ¹  -50.00% (p=0.008 n=5)
ToFileNameLowerCase//user/UserName/proje...etc#01-12   48.00 ± ∞ ¹   48.00 ± ∞ ¹        ~ (p=1.000 n=5) ²
ToFileNameLowerCase//user/UserName/proje...etc#02-12   48.00 ± ∞ ¹   48.00 ± ∞ ¹        ~ (p=1.000 n=5) ²
ToFileNameLowerCase//user/UserName/proje...etc#03-12   48.00 ± ∞ ¹   48.00 ± ∞ ¹        ~ (p=1.000 n=5) ²
ToFileNameLowerCase/FoO/FoO/FoO/FoO/FoO/...etc-12      832.0 ± ∞ ¹   416.0 ± ∞ ¹  -50.00% (p=0.008 n=5)
geomean                                                          ³                -29.29%               ³
¹ need >= 6 samples for confidence interval at level 0.95
² all samples are equal
³ summaries must be >0 to compute geomean

                                                     │   old.txt   │               new.txt                │
                                                     │  allocs/op  │  allocs/op   vs base                 │
ToFileNameLowerCase//path/to/file.ext-12               0.000 ± ∞ ¹   0.000 ± ∞ ¹        ~ (p=1.000 n=5) ²
ToFileNameLowerCase//PATH/TO/FILE.EXT-12               2.000 ± ∞ ¹   1.000 ± ∞ ¹  -50.00% (p=0.008 n=5)
ToFileNameLowerCase//path/to/FILE.EXT-12               2.000 ± ∞ ¹   1.000 ± ∞ ¹  -50.00% (p=0.008 n=5)
ToFileNameLowerCase//user/UserName/proje...etc-12      2.000 ± ∞ ¹   1.000 ± ∞ ¹  -50.00% (p=0.008 n=5)
ToFileNameLowerCase//user/UserName/proje...etc#01-12   1.000 ± ∞ ¹   1.000 ± ∞ ¹        ~ (p=1.000 n=5) ²
ToFileNameLowerCase//user/UserName/proje...etc#02-12   1.000 ± ∞ ¹   1.000 ± ∞ ¹        ~ (p=1.000 n=5) ²
ToFileNameLowerCase//user/UserName/proje...etc#03-12   1.000 ± ∞ ¹   1.000 ± ∞ ¹        ~ (p=1.000 n=5) ²
ToFileNameLowerCase/FoO/FoO/FoO/FoO/FoO/...etc-12      2.000 ± ∞ ¹   1.000 ± ∞ ¹  -50.00% (p=0.008 n=5)
geomean                                                          ³                -29.29%               ³
¹ need >= 6 samples for confidence interval at level 0.95
² all samples are equal
³ summaries must be >0 to compute geomean

I left details in each commit msg about the optimization that's being applied, as well as a perf diff for that specific commit.

  1. No more tempoary []byte copies of the string
  2. Hoisted length to a local fileNameLen
  3. zero copy []byte->string using unsafe.String

Very interesting go fact, having come from rust, the below two go snippets are not equivalent. In the first len(foo) appears to be evaluated on every iteration of the loop, whereas in the second, it's only evaulated once. (I assumed the compiler would hoist the len(foo) in the first case).

for i := range len(foo) {}
fooLen := len(foo)
for i := range fooLen {}

@camc314
Copy link
Contributor Author

camc314 commented Aug 13, 2025

Before (top) and after (bottom) while profiling tsgolint

Screenshot 2025-08-13 at 13 37 33

// string becomes the only live reference to its backing array. The array is
// heap-allocated (via make), so the GC keeps it alive for the lifetime of the
// returned string. Since len(b) > 0 here, &b[0] is a valid pointer.
return unsafe.String(&b[0], len(b))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This unsafe doesn't seem required; I think a strings.Builder and Grow would achieve the same thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried strings.Builder, but it's much slower, let me grab a bench comparing the two.

Completly understand if you dont want the unsafe in here though

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(in general we avoid unsafe as much as possible; there's maybe two places we do it and I'm not particularly thrilled by either)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Go 1.25, the compiler will actually allocate some space on the stack for small arrays, so I'm not 100% sure this is safe anyway. But maybe it detects the escape...

Copy link
Contributor Author

@camc314 camc314 Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bench results
go test -run=- -bench='BenchmarkToFileNameLowerCase' -count=5 ./internal/tspath
goos: darwin
goarch: arm64
pkg: github.com/microsoft/typescript-go/internal/tspath
cpu: Apple M2 Max
BenchmarkToFileNameLowerCase/unsafe.String/path/to/file.ext-12          141487891                8.428 ns/op           0 B/op         0 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/path/to/file.ext-12          142436714                8.389 ns/op           0 B/op         0 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/path/to/file.ext-12          144525740                8.332 ns/op           0 B/op         0 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/path/to/file.ext-12          142487515                8.421 ns/op           0 B/op         0 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/path/to/file.ext-12          142214096                8.528 ns/op           0 B/op         0 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/path/to/file.ext-12          131799111                8.945 ns/op           0 B/op          0 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/path/to/file.ext-12          141256605                8.488 ns/op           0 B/op          0 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/path/to/file.ext-12          142093312                8.463 ns/op           0 B/op          0 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/path/to/file.ext-12          139018815                8.597 ns/op           0 B/op          0 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/path/to/file.ext-12          135752313                8.731 ns/op           0 B/op          0 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/PATH/TO/FILE.EXT-12          43170902                27.71 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/PATH/TO/FILE.EXT-12          43733040                27.73 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/PATH/TO/FILE.EXT-12          43673287                27.91 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/PATH/TO/FILE.EXT-12          43002147                27.71 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/PATH/TO/FILE.EXT-12          42304352                27.86 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/PATH/TO/FILE.EXT-12          26166950                45.41 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/PATH/TO/FILE.EXT-12          26585089                45.70 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/PATH/TO/FILE.EXT-12          25585399                45.51 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/PATH/TO/FILE.EXT-12          26275957                45.51 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/PATH/TO/FILE.EXT-12          26061861                45.43 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/path/to/FILE.EXT-12          39528078                30.41 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/path/to/FILE.EXT-12          39183885                30.72 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/path/to/FILE.EXT-12          38884972                30.61 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/path/to/FILE.EXT-12          38705514                30.19 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/path/to/FILE.EXT-12          39946737                30.10 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/path/to/FILE.EXT-12          27097210                44.67 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/path/to/FILE.EXT-12          26072149                44.61 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/path/to/FILE.EXT-12          26717058                44.45 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/path/to/FILE.EXT-12          26669036                44.48 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/path/to/FILE.EXT-12          26848740                44.83 ns/op           24 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc-12                 25453432                47.47 ns/op           48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc-12                 25446798                46.69 ns/op           48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc-12                 25690887                46.74 ns/op           48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc-12                 25378093                46.95 ns/op           48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc-12                 24890541                46.88 ns/op           48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc-12                 11906872               101.1 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc-12                 11939253               101.3 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc-12                 11911353               101.3 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc-12                 11860124               101.4 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc-12                 11837811               102.2 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc#01-12               7411602               160.0 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc#01-12               7431421               160.5 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc#01-12               7428858               160.4 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc#01-12               7451360               160.1 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc#01-12               7415942               160.4 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc#01-12               7359129               161.1 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc#01-12               7298068               160.4 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc#01-12               7315552               162.4 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc#01-12               7363796               164.1 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc#01-12               7016052               164.3 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc#02-12               7846975               152.7 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc#02-12               7867706               154.1 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc#02-12               7717936               156.0 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc#02-12               7734438               156.6 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc#02-12               7725684               154.1 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc#02-12               7759821               154.3 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc#02-12               7689603               158.0 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc#02-12               7546063               156.0 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc#02-12               7838644               153.9 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc#02-12               7784214               157.4 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc#03-12               8686771               144.2 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc#03-12               8610394               139.6 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc#03-12               8624308               142.6 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc#03-12               8747455               180.8 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.String/user/UserName/proje...etc#03-12               8615234               137.7 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc#03-12               8757932               136.7 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc#03-12               8846365               136.1 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc#03-12               8766225               136.9 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc#03-12               8735162               137.4 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilder/user/UserName/proje...etc#03-12               8869804               137.8 ns/op            48 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.StringFoO/FoO/FoO/FoO/FoO/...etc-12                  2328634               509.4 ns/op           416 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.StringFoO/FoO/FoO/FoO/FoO/...etc-12                  2366596               509.3 ns/op           416 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.StringFoO/FoO/FoO/FoO/FoO/...etc-12                  2378744               503.9 ns/op           416 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.StringFoO/FoO/FoO/FoO/FoO/...etc-12                  2335215               507.6 ns/op           416 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/unsafe.StringFoO/FoO/FoO/FoO/FoO/...etc-12                  2385950               503.2 ns/op           416 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilderFoO/FoO/FoO/FoO/FoO/...etc-12                  1000000              1053 ns/op             416 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilderFoO/FoO/FoO/FoO/FoO/...etc-12                  1000000              1046 ns/op             416 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilderFoO/FoO/FoO/FoO/FoO/...etc-12                  1000000              1062 ns/op             416 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilderFoO/FoO/FoO/FoO/FoO/...etc-12                  1000000              1045 ns/op             416 B/op          1 allocs/op
BenchmarkToFileNameLowerCase/StringBuilderFoO/FoO/FoO/FoO/FoO/...etc-12                  1000000              1052 ns/op             416 B/op          1 allocs/op
Analysis courtercy of ChatGPT:

Short version: strings.Builder is consistently slower here, especially when many bytes need changing. Your []byte + unsafe.String path wins across the board.

What the numbers say

  • All-lowercase ASCII (path/to/file.ext)
    • unsafe: ~8.42 ns/op
    • builder: ~8.64 ns/op (≈ +3%)
    • Both 0 allocs. Minor call/branch overhead in Builder shows up even on the no-change path.
  • All-uppercase ASCII (PATH/TO/FILE.EXT)
    • unsafe: ~27.8 ns/op
    • builder: ~45.5 ns/op (≈ +64%)
    • 1 alloc (~24 B) in both (new string). Big gap once you’re writing/transforming every byte.
  • Mixed case (path/to/FILE.EXT)
    • unsafe: ~30.4 ns/op
    • builder: ~44.6 ns/op (≈ +47%)
    • Same story.
  • Long mixed path (“user/UserName/…etc”)
    • unsafe: ~46.8–47.5 ns/op
    • builder: ~101–102 ns/op (≈ 2.1× slower)
  • Unicode-edge tests (Initial port of compiler #1, Use gofmt -s for formatting #2, Change parseList funcs into methods, use method expressions in calls #3)
    • Results vary a bit, but builder is never better; usually a touch slower or similar.
    • Both show 1 alloc (~48 B). (Likely due to hitting the slow path / constructing a new string segment.)
  • Very long repetitive path (FoO/×100)
    -unsafe: ~504–509 ns/op
    -builder: ~1045–1062 ns/op (≈ 2.06× slower)
    -1 alloc (~416 B) in both. Worst case amplifies Builder overhead.

Why Builder loses here

  • strings.Builder.WriteByte still does a capacity check per write; Grow prevents reallocation but not the per-call branch.
  • Method call + bounds/capacity check overhead, even if inlined, adds up over tens/hundreds of bytes.
  • Your []byte loop is extremely cheap; the compiler often hoists/elides bounds checks in the b[i] = c loop.
  • unsafe.String(&b[0], len(b)) avoids the byte→string copy on return. (Builder’s String() is also zero-copy, but the per-write overhead already sunk it.)

Takeaways / recommendations

  • Keep the current ASCII fast path with []byte + unsafe.String. It’s the fastest and still 0 allocs for “no-change” inputs, with a single alloc when you must lower.
  • If you want a fully safe version (no unsafe) as a fallback or for paranoia builds, prefer the []byte approach with return string(b) over strings.Builder; it’ll still beat Builder in these tight loops.
  • Minor micro-tweaks (optional):
  • Cache n := len(fileName) (you already did).
  • Keep the for i := 0; i < n; i++ { c := fileName[i] … } form; it tends to produce excellent codegen and bounds-check elimination.
  • Your 'A'..'Z' check + c += 'a' - 'A' is already optimal for ASCII. (Bit-trick c |= 0x20 still needs the range guard, so no gain.)

Bottom line: don’t switch to strings.Builder for this kind of tight ASCII transformation. Your current unsafe.String path is measurably superior, especially on uppercase-heavy and long inputs.

tldr: String.Builder is ~1.5-2x slower depending on the scenario vs unsafe.String

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though I will note that (2) isn't something to worry about, as Go does not have uninitialized memory; the slice will be all zeros after being allocated.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

goos: linux
goarch: amd64
pkg: github.com/microsoft/typescript-go/internal/tspath
cpu: Intel(R) Core(TM) i9-10900K CPU @ 3.70GHz
                                                     │   old.txt   │               new.txt               │              new2.txt               │
                                                     │   sec/op    │   sec/op     vs base                │   sec/op     vs base                │
ToFileNameLowerCase//path/to/file.ext-20               9.450n ± 1%   9.719n ± 1%   +2.85% (p=0.000 n=10)   9.549n ± 1%   +1.04% (p=0.019 n=10)
ToFileNameLowerCase//PATH/TO/FILE.EXT-20               43.46n ± 1%   36.45n ± 1%  -16.15% (p=0.000 n=10)   48.50n ± 1%  +11.58% (p=0.000 n=10)
ToFileNameLowerCase//path/to/FILE.EXT-20               44.25n ± 1%   39.50n ± 0%  -10.73% (p=0.000 n=10)   47.41n ± 1%   +7.13% (p=0.000 n=10)
ToFileNameLowerCase//user/UserName/proje...etc-20      93.23n ± 1%   58.94n ± 1%  -36.79% (p=0.000 n=10)   85.21n ± 1%   -8.61% (p=0.000 n=10)
ToFileNameLowerCase//user/UserName/proje...etc#01-20   216.0n ± 1%   204.7n ± 0%   -5.21% (p=0.000 n=10)   209.3n ± 1%   -3.08% (p=0.000 n=10)
ToFileNameLowerCase//user/UserName/proje...etc#02-20   208.3n ± 1%   200.1n ± 1%   -3.96% (p=0.000 n=10)   203.5n ± 1%   -2.33% (p=0.000 n=10)
ToFileNameLowerCase//user/UserName/proje...etc#03-20   182.7n ± 0%   175.9n ± 1%   -3.72% (p=0.000 n=10)   178.0n ± 1%   -2.57% (p=0.000 n=10)
ToFileNameLowerCase/FoO/FoO/FoO/FoO/FoO/...etc-20      919.7n ± 1%   760.9n ± 0%  -17.26% (p=0.000 n=10)   710.8n ± 1%  -22.72% (p=0.000 n=10)
geomean                                                103.1n        90.55n       -12.21%                  100.1n        -2.95%

                                                     │   old.txt    │               new.txt                │               new2.txt               │
                                                     │     B/op     │    B/op     vs base                  │    B/op     vs base                  │
ToFileNameLowerCase//path/to/file.ext-20               0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹   0.000 ± 0%        ~ (p=1.000 n=10) ¹
ToFileNameLowerCase//PATH/TO/FILE.EXT-20               24.00 ± 0%     24.00 ± 0%        ~ (p=1.000 n=10) ¹   24.00 ± 0%        ~ (p=1.000 n=10) ¹
ToFileNameLowerCase//path/to/FILE.EXT-20               24.00 ± 0%     24.00 ± 0%        ~ (p=1.000 n=10) ¹   24.00 ± 0%        ~ (p=1.000 n=10) ¹
ToFileNameLowerCase//user/UserName/proje...etc-20      96.00 ± 0%     48.00 ± 0%  -50.00% (p=0.000 n=10)     48.00 ± 0%  -50.00% (p=0.000 n=10)
ToFileNameLowerCase//user/UserName/proje...etc#01-20   48.00 ± 0%     48.00 ± 0%        ~ (p=1.000 n=10) ¹   48.00 ± 0%        ~ (p=1.000 n=10) ¹
ToFileNameLowerCase//user/UserName/proje...etc#02-20   48.00 ± 0%     48.00 ± 0%        ~ (p=1.000 n=10) ¹   48.00 ± 0%        ~ (p=1.000 n=10) ¹
ToFileNameLowerCase//user/UserName/proje...etc#03-20   48.00 ± 0%     48.00 ± 0%        ~ (p=1.000 n=10) ¹   48.00 ± 0%        ~ (p=1.000 n=10) ¹
ToFileNameLowerCase/FoO/FoO/FoO/FoO/FoO/...etc-20      832.0 ± 0%     416.0 ± 0%  -50.00% (p=0.000 n=10)     416.0 ± 0%  -50.00% (p=0.000 n=10)
geomean                                                           ²               -15.91%                ²               -15.91%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

                                                     │   old.txt    │               new.txt                │               new2.txt               │
                                                     │  allocs/op   │ allocs/op   vs base                  │ allocs/op   vs base                  │
ToFileNameLowerCase//path/to/file.ext-20               0.000 ± 0%     0.000 ± 0%        ~ (p=1.000 n=10) ¹   0.000 ± 0%        ~ (p=1.000 n=10) ¹
ToFileNameLowerCase//PATH/TO/FILE.EXT-20               1.000 ± 0%     1.000 ± 0%        ~ (p=1.000 n=10) ¹   1.000 ± 0%        ~ (p=1.000 n=10) ¹
ToFileNameLowerCase//path/to/FILE.EXT-20               1.000 ± 0%     1.000 ± 0%        ~ (p=1.000 n=10) ¹   1.000 ± 0%        ~ (p=1.000 n=10) ¹
ToFileNameLowerCase//user/UserName/proje...etc-20      2.000 ± 0%     1.000 ± 0%  -50.00% (p=0.000 n=10)     1.000 ± 0%  -50.00% (p=0.000 n=10)
ToFileNameLowerCase//user/UserName/proje...etc#01-20   1.000 ± 0%     1.000 ± 0%        ~ (p=1.000 n=10) ¹   1.000 ± 0%        ~ (p=1.000 n=10) ¹
ToFileNameLowerCase//user/UserName/proje...etc#02-20   1.000 ± 0%     1.000 ± 0%        ~ (p=1.000 n=10) ¹   1.000 ± 0%        ~ (p=1.000 n=10) ¹
ToFileNameLowerCase//user/UserName/proje...etc#03-20   1.000 ± 0%     1.000 ± 0%        ~ (p=1.000 n=10) ¹   1.000 ± 0%        ~ (p=1.000 n=10) ¹
ToFileNameLowerCase/FoO/FoO/FoO/FoO/FoO/...etc-20      2.000 ± 0%     1.000 ± 0%  -50.00% (p=0.000 n=10)     1.000 ± 0%  -50.00% (p=0.000 n=10)
geomean                                                           ²               -15.91%                ²               -15.91%                ²
¹ all samples are equal
² summaries must be >0 to compute geomean

With strings.Builder. Pretty surprised by this. I really don't want to have to use unsafe here ☹️

Copy link
Contributor Author

@camc314 camc314 Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jakebailey shall i split this PR into two to help it get merged quicker?

one (i'll keep this one for the GH convo), with the unsafe change, and the other just with the first commit?

i think the combination of the two commits in the above bench is making the true perf changes in using unsafe vs string builder harder to see.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, yes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Just update the PR title/description to match what the PR contains)

@camc314 camc314 force-pushed the c/optimize-to-file-name-lower-case-again branch from cdf15cd to 7db11f5 Compare August 13, 2025 12:51
@jakebailey
Copy link
Member

jakebailey commented Aug 13, 2025

Very interesting go fact, having come from rust, the below two go snippets are not equivalent. In the first len(foo) appears to be evaluated on every iteration of the loop, whereas in the second, it's only evaulated once. (I assumed the compiler would hoist the len(foo) in the first case).

That isn't my understanding; https://go.dev/play/p/eTayG5--ykW iterates 5 times even though the len changes. Maybe you've hit a compiler bug or missing optimization?

@camc314
Copy link
Contributor Author

camc314 commented Aug 13, 2025

Very interesting go fact, having come from rust, the below two go snippets are not equivalent. In the first len(foo) appears to be evaluated on every iteration of the loop, whereas in the second, it's only evaulated once. (I assumed the compiler would hoist the len(foo) in the first case).

That isn't my understanding; https://go.dev/play/p/eTayG5--ykW iterates 5 times even though the len changes. Maybe you've hit a compiler bug or missing optimization?

hmm that's very strange yeah quite possibly a missing optimization or a bug somewhere.

@camc314
Copy link
Contributor Author

camc314 commented Aug 13, 2025

Will rebase this once #1581 is merged

Use unsafe.String to convert the lowercase byte slice to a string
without an extra allocation and copy, reducing overhead in the ASCII
fast path.

```
                                                     │ bench-to-file-name-lower-case-BASE.txt │ bench-to-file-name-lower-case-with-unsafe-string.txt │
                                                     │                 sec/op                 │            sec/op              vs base               │
ToFileNameLowerCase//path/to/file.ext-12                                         8.348n ± ∞ ¹                    8.505n ± ∞ ¹        ~ (p=0.151 n=5)
ToFileNameLowerCase//PATH/TO/FILE.EXT-12                                         37.94n ± ∞ ¹                    27.80n ± ∞ ¹  -26.73% (p=0.008 n=5)
ToFileNameLowerCase//path/to/FILE.EXT-12                                         40.96n ± ∞ ¹                    30.67n ± ∞ ¹  -25.12% (p=0.008 n=5)
ToFileNameLowerCase//user/UserName/proje...etc-12                                58.11n ± ∞ ¹                    46.78n ± ∞ ¹  -19.50% (p=0.008 n=5)
ToFileNameLowerCase//user/UserName/proje...etc#01-12                             160.7n ± ∞ ¹                    161.3n ± ∞ ¹        ~ (p=0.286 n=5)
ToFileNameLowerCase//user/UserName/proje...etc#02-12                             155.5n ± ∞ ¹                    155.9n ± ∞ ¹        ~ (p=0.381 n=5)
ToFileNameLowerCase//user/UserName/proje...etc#03-12                             138.1n ± ∞ ¹                    137.8n ± ∞ ¹        ~ (p=0.421 n=5)
ToFileNameLowerCase/FoO/FoO/FoO/FoO/FoO/...etc-12                                543.2n ± ∞ ¹                    515.3n ± ∞ ¹   -5.14% (p=0.008 n=5)
geomean                                                                          78.30n                          70.43n        -10.05%
¹ need >= 6 samples for confidence interval at level 0.95
```
@camc314 camc314 force-pushed the c/optimize-to-file-name-lower-case-again branch from 7db11f5 to a1f7130 Compare August 14, 2025 09:01
@camc314 camc314 changed the title perf(tspath): optimize ToFileNameLowerCase again perf(tspath): avoid string copy in ToFileNameLowerCase Aug 14, 2025
@camc314 camc314 marked this pull request as ready for review August 14, 2025 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants