Skip to content

Optimize internal extract! calls to save on memory allocation #7

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.

Sign up for GitHub

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

Merged
moberegger merged 2 commits into from
Jun 2, 2025
Merged

Optimize internal extract! calls to save on memory allocation #7

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
25f48f4
Optimize extract to save on memory allocation
moberegger May 30, 2025
0622464
Back to fetch
moberegger Jun 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions lib/jbuilder.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def set!(key, value = BLANK, *args, &block)
else
# json.author @post.creator, :name, :email_address
# { "author": { "name": "David", "email_address": "[email protected]" } }
_merge_block(key){ extract! value, *args }
_merge_block(key){ _extract value, args }
end

_set_value key, result
Expand Down Expand Up @@ -215,7 +215,7 @@ def array!(collection = [], *attributes, &block)
elsif ::Kernel.block_given?
_map_collection(collection, &block)
elsif attributes.any?
_map_collection(collection) { |element| extract! element, *attributes }
_map_collection(collection) { |element| _extract element, attributes }
else
_format_keys(collection.to_a)
end
Expand All @@ -241,18 +241,14 @@ def array!(collection = [], *attributes, &block)
#
# json.(@person, :name, :age)
def extract!(object, *attributes)
if ::Hash === object
_extract_hash_values(object, attributes)
else
_extract_method_values(object, attributes)
end
_extract object, attributes
end

def call(object, *attributes, &block)
if ::Kernel.block_given?
array! object, &block
else
extract! object, *attributes
_extract object, attributes
end
end

Expand Down Expand Up @@ -281,8 +277,16 @@ def target!

private

def _extract(object, attributes)
if ::Hash === object
_extract_hash_values(object, attributes)
else
_extract_method_values(object, attributes)
end
end

def _extract_hash_values(object, attributes)
attributes.each{ |key| _set_value key, _format_keys(object.fetch(key)) }
attributes.each{ |key| _set_value key, _format_keys(object[key]) }
Copy link
Author

@moberegger moberegger May 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also using [] instead of fetch. It's a bit faster.

ruby 3.4.4 (2025-05-14 revision a38531fd3f) +YJIT +PRISM [arm64-darwin24]
Warming up --------------------------------------
                  []     1.582M i/100ms
               fetch     1.627M i/100ms
Calculating -------------------------------------
                  []     26.015M (± 4.2%) i/s   (38.44 ns/i) -    129.730M in   4.999980s
               fetch     21.967M (± 4.2%) i/s   (45.52 ns/i) -    110.641M in   5.047802s

Comparison:
                  []: 26014972.6 i/s
               fetch: 21966990.7 i/s - 1.18x  slower

Copy link

@Insomniak47 Insomniak47 May 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the behaviour identical on missing keys?

Copy link
Author

@moberegger moberegger Jun 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question. fetch actually throws if the key doesn't exist, vs returning nil. Not clear if this is the intended behaviour or just happenstance. The coding style for the project seems to prefer fetch. I git blamed back 10+ years to see if there was any reasoning for this, but saw nothing of note before losing interest.

Mixed feelings on this. The current implementation would result in 500 errors if/when this occurs, which is not something I would expect or desire from our tooling. But given my intent here was simply performance - not a behavioural change - I will switch this back to fetch; the overall savings still fall within the margin of error (according to ips) and not worth the risk and drift from the upstream project.

end

def _extract_method_values(object, attributes)
Expand Down