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

Use set data structure to speed up circular reference checks on large/deeply nested objects #1239

Merged
merged 1 commit into from
May 24, 2011

Conversation

alindeman
Copy link
Contributor

This is a refactor, so no updates to tests.

This also could be cherry picked cleanly into 3-1-stable.

@josevalim
Copy link
Contributor

Do you have some simple benchmark that shows that using set is really faster? Thanks!

@alindeman
Copy link
Contributor Author

Given (I've seen objects even "bigger" than this need to be encoded for certain special cases):

big_object = 500.times.map { |i| Hash[('a'..'z').map { |j| [j, j] }] }
ruby-1.9.2-p180 :016 > Benchmark.bm do |x|
ruby-1.9.2-p180 :017 >     x.report('array') { 100.times { ActiveSupport::JSON.encode(big_object) } }
ruby-1.9.2-p180 :018?>   end
      user     system      total        real
array 28.600000   0.100000  28.700000 ( 28.703123)
ruby-1.9.2-p180 :011 > Benchmark.bm do |x|
ruby-1.9.2-p180 :012 >     x.report('set') { 100.times { ActiveSupport::JSON.encode(big_object) } }
ruby-1.9.2-p180 :013?>   end
      user     system      total        real
set 27.440000   0.110000  27.550000 ( 27.546195)

While the numbers varied slightly over multiple runs, set always beat array by a significant enough margin for me to think this small patch is worth applying.

josevalim added a commit that referenced this pull request May 24, 2011
Use set data structure to speed up circular reference checks on large/deeply nested objects
@josevalim josevalim merged commit b3f51e3 into rails:master May 24, 2011
jake3030 pushed a commit to jake3030/rails that referenced this pull request Jun 28, 2011
… users with the gem installed not seeing our changes.

The changes will be submitted upstream.
Signed-off-by: Michael Koziarski <michael@koziarski.com>
[rails#1239 state:committed]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants