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

Make AM Serializar at least three times faster #230

Closed
SamSaffron opened this issue Mar 14, 2013 · 2 comments
Closed

Make AM Serializar at least three times faster #230

SamSaffron opened this issue Mar 14, 2013 · 2 comments

Comments

@SamSaffron
Copy link
Contributor

While profiling Discourse with DTRACE I have notices a very large amount of time is being spent inside AM::Serializer iterating through hashes etc.

My fork is at: https://github.com/SamSaffron/active_model_serializers

I forked and patched it up so its AT LEAST twice faster ... and a lot more for larger serializer, results running against Ruby 2.0 are here (benchmark is in the benchmark folder)

                                             user     system      total        real
init                                     0.060000   0.000000   0.060000 (  0.061526)
fast_hash                                0.070000   0.000000   0.070000 (  0.066898)
fast_attributes                          0.260000   0.000000   0.260000 (  0.264070)
fast_serializable_hash                   0.350000   0.000000   0.350000 (  0.356954)
attributes                               0.470000   0.000000   0.470000 (  0.473984)
serializable_hash                        0.580000   0.000000   0.580000 (  0.573823)
serializable_hash_with_instrumentation   0.980000   0.000000   0.980000 (  0.983716)

The "fast_attributes" method is a direct drop in:

    def fast_attributes
      _fast_attributes
      rescue NameError
        method = "def _fast_attributes\n"

        method << "h = {}\n"

        INCLUDE_METHODS.each do |k,v|
          method << "h[:#{k}] = read_attribute_for_serialization(:#{k}) if #{v}\n"
        end
        method << "h\nend"

        self.class.class_eval method
        fast_attributes
    end

The other fix which has MAJOR perf benefits is stripping out the excessive instrumentation.

My recommendation here would be not to be doing any instrumentation in

def serializable_hash
      return nil if @object.nil?
      instrument(:serialize, :serializer => self.class.name) do
        @node = attributes
        instrument :associations do
          include_associations! if _embed
        end
        @node
      end
    end

If people would like instrumentation let them opt in with.

ActiveModel::Serializer.enable_instrumentation!


With both fixes in place real time goes down from 983ms to 356ms ... a pretty significant change.


Let me know if you are ok with both and I will turn this into a PR.

@SamSaffron
Copy link
Contributor Author

Note, there is still room for improvement, the "if" checks can be omitted in the serializer generated if code is restructures a bit, also I think a method call can be erased. eg go directly to the object instead of bridging for certain cases.

@steveklabnik
Copy link
Contributor

Please do send a pull. ❤️

On Mar 13, 2013, at 6:43 PM, Sam notifications@github.com wrote:

Note, there is still room for improvement, the "if" checks can be omitted in the serializer generated if code is restructures a bit, also I think a method call can be erased. eg go directly to the object instead of bridging for certain cases.


Reply to this email directly or view it on GitHub.

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

No branches or pull requests

2 participants