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

Optimize URI.decode #11741

Merged
merged 5 commits into from
Jan 21, 2022
Merged

Optimize URI.decode #11741

merged 5 commits into from
Jan 21, 2022

Conversation

asterite
Copy link
Member

This forum post hints that URI.decode is slow, so I checked if that's the case... and it is!

It seems that #11124 improved URI.decode to take into account writing into an IO that doesn't have the UTF-8 encoding. That's great! But it also made the most usual case, UTF-8, slower, by always creating an intermediate IO::Memory.

The first commits improves that: if it's UTF-8 we don't need to use an intermediate IO (I think! Please correct me if I'm wrong.)

The second commit is a general optimization: URI.decode translates %xx and sometimes a + to a space, and those are the only sequences that needs a transformation. That also means that if none of those chars (% and +) appear in the string, there's no need to decode anything. For the URI.decode(string) case it means we can directly return string, without allocating any memory at all. For the URI.decode(string, io) case it means we don't need to go byte by byte: we can directly write the string into the IO.

Now, checking whether the % or + chars appear in a string means traversing the entire string, so in theory it adds a tiny overhead when decoding is actually needed, but:

  • I think it's a good compromise
  • searching those chars will end up translating to memchr, which is extremely fast
  • and in any case, this PR makes everything so much faster than before, so I think this extra penalty is good if we were using slower code before and nobody complained

Here's a benchmark:

require "uri"
require "benchmark"

io = IO::Memory.new

Benchmark.ips do |x|
  x.report("decode without escapes") do
    URI.decode("hello abc world")
  end
  x.report("decode with escape") do
    URI.decode("hello world %31")
  end
  x.report("decode without escapes (IO)") do
    URI.decode("hello abc world", io)
    io.clear
  end
  x.report("decode with escape (IO)") do
    URI.decode("hello world %31", io)
    io.clear
  end
end

Before:

     decode without escapes   5.02M (199.16ns) (± 8.32%)  320B/op
         decode with escape   5.37M (186.19ns) (± 5.27%)  320B/op
decode without escapes (IO)   9.27M (107.93ns) (± 4.20%)  112B/op
    decode with escape (IO)   9.62M (103.98ns) (± 3.19%)  112B/op

After:

     decode without escapes 109.58M (  9.13ns) (± 5.46%)  0.0B/op
         decode with escape   7.09M (141.11ns) (± 4.52%)  208B/op
decode without escapes (IO)  62.71M ( 15.95ns) (± 4.94%)  0.0B/op
    decode with escape (IO)  16.09M ( 62.16ns) (± 3.39%)  0.0B/op

Notice that the most common case of URI.decode(string) where no decoding is needed improves by like 20 times.

src/uri/encoding.cr Outdated Show resolved Hide resolved
@beta-ziliani beta-ziliani added this to the 1.4.0 milestone Jan 12, 2022
@straight-shoota straight-shoota changed the title Optimize URI.decode Optimize URI.decode Jan 20, 2022
@straight-shoota straight-shoota merged commit 8ef1361 into master Jan 21, 2022
@straight-shoota straight-shoota deleted the opt/uri-decode branch January 21, 2022 17:20
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.

5 participants