Skip to content

Commit 18cb909

Browse files
p8byroot
authored andcommitted
Don't parse response content type if value comes from Mime
Anytime `ActionDispatch::HTTP::Reponse#content_type=` is called, the `content_type` argument is parsed using the CONTENT_TYPE_PARSER regexp. If the `content_type` argument comes from `Mime`, there is no need to parse it using the regexp as we already know the result. ```ruby require "bundler/inline" gemfile(true) do source "https://rubygems.org" gem "rails" gem "benchmark-ips" end require "action_controller/railtie" require 'benchmark/ips' Benchmark.ips do |x| x.config(:time => 5, :warmup => 2) response = ActionDispatch::Response.new x.report("main") do response.content_type = Mime[:html].to_s end class BranchResponse < ActionDispatch::Response def content_type=(content_type) case content_type when NilClass return when Symbol new_header_info = ContentTypeHeader.new(Mime[content_type]) else new_header_info = parse_content_type(content_type.to_s) end prev_header_info = parsed_content_type_header charset = new_header_info.charset || prev_header_info.charset charset ||= self.class.default_charset unless prev_header_info.mime_type set_content_type new_header_info.mime_type, charset end end response = BranchResponse.new x.report("branch") do response.content_type = :html end x.compare! end ``` ``` Warming up -------------------------------------- main 38.666k i/100ms branch 52.235k i/100ms Calculating ------------------------------------- main 393.529k (± 3.3%) i/s (2.54 μs/i) - 1.972M in 5.016529s branch 494.789k (± 2.2%) i/s (2.02 μs/i) - 2.507M in 5.069774s Comparison: branch: 494788.6 i/s main: 393529.2 i/s - 1.26x slower ``` This improves the plain json call in the TechEmpower benchmark by reducing the time spent in `#assign_default_content_type_and_charset!` from 16% to 6.5%. It probably improves other calls as well.
1 parent c165c07 commit 18cb909

File tree

7 files changed

+45
-12
lines changed

7 files changed

+45
-12
lines changed

actionpack/CHANGELOG.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,13 @@
1+
* Allow setting content type with a symbol of the Mime type.
2+
3+
```ruby
4+
# Before
5+
response.content_type = "text/html"
6+
7+
# After
8+
response.content_type = :html
9+
```
10+
11+
*Petrik de Heus*
112

213
Please check [8-0-stable](https://github.com/rails/rails/blob/8-0-stable/actionpack/CHANGELOG.md) for previous changes.

actionpack/lib/action_controller/metal/data_streaming.rb

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,7 @@ def send_file_headers!(options)
132132
raise ArgumentError, ":type option required" if content_type.nil?
133133

134134
if content_type.is_a?(Symbol)
135-
extension = Mime[content_type]
136-
raise ArgumentError, "Unknown MIME type #{options[:type]}" unless extension
137-
self.content_type = extension
135+
self.content_type = content_type
138136
else
139137
if !type_provided && options[:filename]
140138
# If type wasn't provided, try guessing from file extension.

actionpack/lib/action_controller/metal/head.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ def head(status, options = nil)
4141

4242
if include_content?(response_code)
4343
unless self.media_type
44-
self.content_type = content_type || ((f = formats) && Mime[f.first]) || Mime[:html]
44+
self.content_type = content_type || ((f = formats) && Mime[f.first]) || :html
4545
end
4646

4747
response.charset = false

actionpack/lib/action_controller/metal/renderers.rb

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,23 +157,23 @@ def _render_to_body_with_renderer(options)
157157

158158
if options[:callback].present?
159159
if media_type.nil? || media_type == Mime[:json]
160-
self.content_type = Mime[:js]
160+
self.content_type = :js
161161
end
162162

163163
"/**/#{options[:callback]}(#{json})"
164164
else
165-
self.content_type = Mime[:json] if media_type.nil?
165+
self.content_type = :json if media_type.nil?
166166
json
167167
end
168168
end
169169

170170
add :js do |js, options|
171-
self.content_type = Mime[:js] if media_type.nil?
171+
self.content_type = :js if media_type.nil?
172172
js.respond_to?(:to_js) ? js.to_js(options) : js
173173
end
174174

175175
add :xml do |xml, options|
176-
self.content_type = Mime[:xml] if media_type.nil?
176+
self.content_type = :xml if media_type.nil?
177177
xml.respond_to?(:to_xml) ? xml.to_xml(options) : xml
178178
end
179179
end

actionpack/lib/action_controller/metal/rendering.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ def _render_in_priorities(options)
208208
end
209209

210210
def _set_html_content_type
211-
self.content_type = Mime[:html].to_s
211+
self.content_type = :html
212212
end
213213

214214
def _set_rendered_content_type(format)

actionpack/lib/action_dispatch/http/response.rb

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -263,14 +263,27 @@ def status=(status)
263263
# Sets the HTTP response's content MIME type. For example, in the controller you
264264
# could write this:
265265
#
266-
# response.content_type = "text/plain"
266+
# response.content_type = "text/html"
267+
#
268+
# This method also accepts a symbol with the extension of the MIME type:
269+
#
270+
# response.content_type = :html
267271
#
268272
# If a character set has been defined for this response (see #charset=) then the
269273
# character set information will also be included in the content type
270274
# information.
271275
def content_type=(content_type)
272-
return unless content_type
273-
new_header_info = parse_content_type(content_type.to_s)
276+
case content_type
277+
when NilClass
278+
return
279+
when Symbol
280+
mime_type = Mime[content_type]
281+
raise ArgumentError, "Unknown MIME type #{content_type}" unless mime_type
282+
new_header_info = ContentTypeHeader.new(mime_type.to_s)
283+
else
284+
new_header_info = parse_content_type(content_type.to_s)
285+
end
286+
274287
prev_header_info = parsed_content_type_header
275288
charset = new_header_info.charset || prev_header_info.charset
276289
charset ||= self.class.default_charset unless prev_header_info.mime_type

actionpack/test/controller/content_type_test.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,11 @@ def render_content_type_with_charset
5555
response.content_type = "text/html; fragment; charset=utf-16"
5656
render body: "hello world!"
5757
end
58+
59+
def render_content_type_with_symbol
60+
response.content_type = :rss
61+
render body: "hello world!"
62+
end
5863
end
5964

6065
class ContentTypeTest < ActionController::TestCase
@@ -142,6 +147,12 @@ def test_content_type_with_charset
142147
assert_equal "utf-16", @response.charset
143148
end
144149

150+
def test_content_type_with_symbol
151+
get :render_content_type_with_symbol
152+
assert_equal Mime[:rss], @response.media_type
153+
assert_equal "utf-8", @response.charset
154+
end
155+
145156
private
146157
def with_default_charset(charset)
147158
old_default_charset = ActionDispatch::Response.default_charset

0 commit comments

Comments
 (0)