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 by precomputing capacity if possible #8078

Merged
merged 1 commit into from
Aug 14, 2019

Conversation

asterite
Copy link
Member

Nothing spectacular, but still...

Code:

require "benchmark"

base = "/Users/someone/foo/bar"
parts = ["baz", "qux", "foo", "bar", "baz"]

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

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

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

Results:

      join   5.28M (189.53ns) (± 3.96%)  176B/op   1.05× slower
join (new)   5.52M (181.08ns) (± 1.03%)  160B/op        fastest
      join   8.64M (115.69ns) (± 3.00%)  176B/op   1.22× slower
join (new)  10.57M ( 94.59ns) (± 3.49%)  144B/op        fastest

Path#join also has a bug: in one part of the code it calls parts.size but it's not fine to assume that given that it's just Enumerable. But it's probably fine because at least in the std all enumerables have a known size.

src/path.cr Outdated Show resolved Hide resolved
src/path.cr Show resolved Hide resolved
@asterite asterite added this to the 0.31.0 milestone Aug 14, 2019
@asterite asterite merged commit fcd6973 into master Aug 14, 2019
@asterite asterite deleted the opt/path-join branch August 14, 2019 19:14
dnamsons pushed a commit to dnamsons/crystal that referenced this pull request Jan 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants