Fewer allocated objects on each request by schneems · Pull Request #737 · rack/rack

schneems · 2014-10-01T23:36:30Z

How many? Using memory_profiler and a Rails app (codetriage.com), master uses:

rack/lib x 7318

After this patch, the app uses:

rack/lib x 4598

Or (7318 - 4598) / 7318.0 * 100 # => 37.16 % fewer objects PER REQUEST.

To do this, I extracted really commonly used strings into top level Rack constants. It makes for a bit of a big diff, but I believe the changes are worth it.

Running benchmark/ips against the same app, I'm seeing a performance boost of 2~4% across the entire app response. This doesn't just make Rack faster, it will make your app faster.

While we could certainly go overboard and pre-define ALL strings as constants, that would be pretty gnarly to work with. This patch goes after the largest of the low hanging fruit.

nthj · 2014-10-02T04:13:09Z

👍 Yes please.

bbwharris · 2014-10-02T04:18:02Z

👍 makes sense

thedarkone · 2014-10-02T09:39:44Z

I was under impression that literal non-interpolated regexps such as /^to_ary$/ are singletons and don't actually allocate a Regexp object on each execution.

My favorite type of comment on an open source pull request: one that teaches me something i didn't know. You are correct. I tested this out with memory_profiler and funny enough, it doesn't even show a single object allocated

report = MemoryProfiler.report do num.times { foo = /^to_ary$/ } end report.pretty_print Total allocated 0 Total retained 0 allocated memory by gem ----------------------------------- allocated memory by file ----------------------------------- allocated memory by location ----------------------------------- allocated objects by gem ----------------------------------- allocated objects by file ----------------------------------- allocated objects by location ----------------------------------- retained memory by gem ----------------------------------- retained memory by file ----------------------------------- retained memory by location ----------------------------------- retained objects by gem ----------------------------------- retained objects by file ----------------------------------- retained objects by location ----------------------------------- Allocated String Report ----------------------------------- Retained String Report -----------------------------------

btw, I updated to move the regexp out of a constant and back into the method

How many? Using `memory_profiler` and a Rails app (codetriage.com), master uses: ``` rack/lib x 7318 ``` After this patch, the app uses: ``` rack/lib x 4598 ``` Or `(7318 - 4598) / 7318.0 * 100 # => 37.16` % fewer objects __PER REQUEST__. To do this, I extracted really commonly used strings into top level Rack constants. It makes for a bit of a big diff, but I believe the changes are worth it. Running benchmark/ips against the same app, I'm seeing a performance host of `2~4%` across the entire app response. This doesn't just make Rack faster, it will make your app faster. While we could certainly go overboard and pre-define ALL strings as constants, that would be pretty gnarly to work with. This patch goes after the largest of the low hanging fruit.

Less allocated objects on each request

rkh · 2014-10-02T13:35:49Z

Thank you! <3

pedrocarrico · 2014-10-02T14:35:36Z

A big 👍

tak1n · 2014-10-02T14:38:53Z

Nice :)

raggi · 2014-10-02T18:52:00Z

In 1.9, respond_to takes 2 arguments.

Right you are: http://ruby-doc.org/core-2.1.3/Object.html#method-i-respond_to-3F Looks like 1.8.7 & 2+ also take multiple arguments. I did this because the *args causes us to have an extra array allocation. If we match the method signature respond_to?(string, include_all=false) then the include_all would be an extra object. If I remember this only decreased the overall count by around 100. I'm going to open a new PR to revert these changes, I don't think we can do better than the previous case and still maintain compatibility.

Fix is here: #739 sorry about that, should have known better. Thanks for the catch!

Sorry, but false.object_id == 0, include_all in your example only takes a reference slot, not an RObject structure.

Updated in #742 also can get rid of a string allocation in method_missing

@raggi

The `respond_to?` method was optimized for less object creation, unfortunately I also mangled the method signature. This commit reverts the changes introduced in rack#737 to `BodyProxy#respond_to?` and adds tests. See: https://github.com/rack/rack/pull/737/files#r18359323 cc/ @raggi

tjwallace · 2014-10-03T00:04:14Z

Should be frozen?

Yep, good catch. Thanks! Updated in #742

This doesn't include the #respond_to? changes, because they were reverted in rack#739

zaidakram · 2014-10-05T17:35:54Z

Nice Stuff! 👍

stungeye · 2014-10-05T20:04:19Z

👍

thedarkone reviewed Oct 2, 2014
View reviewed changes

schneems force-pushed the schneems/less-objects branch from f226179 to dc53a8c Compare October 2, 2014 12:42

rkh added a commit that referenced this pull request Oct 2, 2014

Merge pull request #737 from schneems/schneems/less-objects

ab172af

Less allocated objects on each request

rkh merged commit ab172af into rack:master Oct 2, 2014

raggi reviewed Oct 2, 2014
View reviewed changes

schneems mentioned this pull request Oct 2, 2014

Fix: respond_to? takes 2 arguments #739

Merged

soulim mentioned this pull request Oct 2, 2014

Follow-up for the pull request #737 #741

Merged

schneems mentioned this pull request Oct 2, 2014

docs for Reporter SamSaffron/memory_profiler#6

Merged

tjwallace reviewed Oct 3, 2014
View reviewed changes

caius added a commit to caius/rack that referenced this pull request Oct 3, 2014

Backport rack#737

a3212e1

This doesn't include the #respond_to? changes, because they were reverted in rack#739

caius added a commit to caius/rack that referenced this pull request Oct 3, 2014

Backport rack#737

d635ad5

This doesn't include the #respond_to? changes, because they were reverted in rack#739

schneems mentioned this pull request Oct 3, 2014

Missed optimizations #742

Merged

u2 mentioned this pull request Mar 31, 2015

Less allocated objects on each request ruby-grape/grape#973

Closed

bquorning mentioned this pull request Aug 7, 2015

Beyond Ludicrous Speed rails/rails#21057

Merged

BenMorganIO mentioned this pull request Aug 16, 2015

freeze some hotspots in ruby ruby/ruby#1002

Closed

marshall-lee mentioned this pull request Jan 22, 2016

Enable frozen_string_literal feature. sinatra/sinatra#1076

Merged

matthewd mentioned this pull request Oct 30, 2018

Why are CONTENT_LENGTH and CONTENT_TYPE different from every other header? #1311

Closed

schneems changed the title ~~Less allocated objects on each request~~ Fewer allocated objects on each request Feb 6, 2020

jaydorsey mentioned this pull request May 18, 2021

frozen_string_literal followup standardrb/standard#299

Closed

Uh oh!

Conversation

schneems commented Oct 1, 2014

Uh oh!

nthj commented Oct 2, 2014

Uh oh!

bbwharris commented Oct 2, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rkh commented Oct 2, 2014

Uh oh!

pedrocarrico commented Oct 2, 2014

Uh oh!

tak1n commented Oct 2, 2014

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zaidakram commented Oct 5, 2014

Uh oh!

stungeye commented Oct 5, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

14 participants