Closed jviide closed 3 months ago
The "refactor:" pull request name prefix was not allowed so I picked "fix:", and changed the commit messages to use the same prefix as well.
We generally squash-merge external PRs unless there's a compelling reason to keep each commit. linting will pass if either the PR title or every commit has a valid conventional prefix.
Don't worry about keeping the commit messages "valid" as we go through this PR, as we'll be using the PR title if and when this lands.
Thanks for the help in reviewing this @kurtextrem. Yes, that comment was referring to s*
which is a classic ReDOS vector. Historically we've used the .split
approach to fix that, because it's rarely in a hot path where performance matters as much as it does here. Good to know that String.replace
is faster.
This pull request optimizes the Range class in the following ways:
.range
string (used by.format()
and.toString()
) lazily. This seems to improve bench-satisfies and bench-subset scores by up to 9%.The external interface for the class stays the same, except for the new internal
.formatted
property used to cache its lazily calculated string.Range#range
property is now also read-only.There is a new test lazy formatting to ensure full test coverage.
The benchmarks bench-satisfies and bench-subset benefit from these changes, sometimes by up to 40%. Other benchmark results seem to stay the same. Here are the affected benchmarks before:
And after: