Skip to content
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

Path: optimize `join` for the case of joining one single part #8082

Merged
merged 2 commits into from Aug 29, 2019

Conversation

@asterite
Copy link
Member

commented Aug 14, 2019

Follow up to #8078

This optimizes Path#join (and File.join) for the most common case:

File.join("prefix", "suffix")

When we just have two pieces to combine it becomes easier to compute the total bytesize needed for the string and we can use String.new instead of String.build, the latter involving an extra allocation for the String::Builder class (and there's also no need to write things though a pointer, check capacity, etc.)

This results in more code, but I think it's good if we optimize for this case because it's so common.

Benchmark code:

require "benchmark"

base = "/Users/someone/foo/bar"
parts = ["baz.cr"]

Benchmark.ips do |x|
  x.report("join") do
    Path.new(base).join(parts)
  end
end

Results:

before: 10.90M ( 91.78ns) (± 2.80%)  144B/op
after:  19.14M ( 52.23ns) (±11.85%)  48.0B/op

That's twice as fast and with a third of the memory used.

Another benchmark (Dir.glob uses a lot of File.join(x, y)):

require "benchmark"

Benchmark.ips do |x|
  x.report("Dir.glob") do
    Dir.glob("*/*.cr") do
    end
  end
end

Results:

before: 1.85k (541.76µs) (± 4.57%)  56.0kB/op
after:  1.89k (528.95µs) (±10.02%)  42.3kB/op

Not a lot of speed improvement because the File.info? call is the bottleneck (but that's solved by #8081), but look at the memory: 42.3kB/op vs. 56kB/op. Huge difference!

@RX14
RX14 approved these changes Aug 28, 2019
src/path.cr Outdated Show resolved Hide resolved
@RX14
RX14 approved these changes Aug 29, 2019
@RX14 RX14 merged commit 9bb2db4 into master Aug 29, 2019
4 of 5 checks passed
4 of 5 checks passed
ci/circleci: test_linux32 There was an infrastructure problem while running your tests
Details
ci/circleci: check_format Your tests passed on CircleCI!
Details
ci/circleci: test_darwin Your tests passed on CircleCI!
Details
ci/circleci: test_linux Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@asterite

This comment has been minimized.

Copy link
Member Author

commented Aug 29, 2019

@RX14 Thank you! 🙏

@RX14 RX14 added this to the 0.31.0 milestone Sep 8, 2019
@straight-shoota straight-shoota deleted the opt/path-join-single branch Sep 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.