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

Improve performance of HTTPHeader#content_type #145

Merged
merged 1 commit into from
Aug 16, 2023

Conversation

technicalpickles
Copy link
Contributor

In the existing implementation, main_type and sub_type would end up being called multiple times potentially.

Instead of doing that, save the result so it can be re-used.

In the existing implementation, `main_type` and `sub_type` would end up
being called multiple times potentially.

Instead of doing that, save the result so it can be re-used.
@technicalpickles
Copy link
Contributor Author

technicalpickles commented Aug 13, 2023

Benchmark:

require 'benchmark/ips'
require 'net/http'

content_type = "application/json; charset=utf-8"

class Current
  include Net::HTTPHeader
  def initialize
    initialize_http_header({"content-type" => "application/json; charset=utf-8"})
  end
end

class Refactored
  include Net::HTTPHeader
  def initialize
    initialize_http_header({"content-type" => "application/json; charset=utf-8"})
  end

  def content_type
    main = main_type()
    return nil unless main

    sub = sub_type
    if sub
      "#{main}/#{sub}"
    else
      main
    end
  end
end

Benchmark.ips do |x|
  x.report "content_type current" do
    Current.new.content_type
  end

  x.report "content_type refactored" do
    Refactored.new.content_type
  end

  x.compare!
end

Result:

Warming up --------------------------------------
content_type current         27.752k i/100ms
content_type refactored   45.404k i/100ms
Calculating -------------------------------------
content_type current         281.205k (± 0.7%) i/s -      1.415M in   5.033402s
content_type refactored   452.947k (± 0.8%) i/s -      2.270M in   5.012417s

Comparison:
content_type refactored:   452946.6 i/s
content_type current:         281205.3 i/s - 1.61x  slower

@yuki24 yuki24 merged commit 3473e27 into ruby:master Aug 16, 2023
12 checks passed
@yuki24
Copy link
Member

yuki24 commented Aug 16, 2023

Thank you!

@technicalpickles technicalpickles deleted the httheader-content_type-perf branch August 16, 2023 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants