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 to_json when rendering json response #181

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alexspeller
Copy link

I was trying to debug why an inertia json response was 5x slower than the initial html response.

After some investigation, it turns out that render json: foo does not actually do the same thing as foo.to_json does. It does a whole lot of rails magic instead, which is both a lot slower and inconsistent.

Changing this to use page.to_json ensures consistency with the props output in inertia.html.erb, avoiding weird inconsistencies between props renders and full page renders, and is also a lot faster (especially if you are using a library like Oj to optimise json generation).

I was trying to debug why an inertia response was 5x slower than the initial page response.

After some investigation, it turns out that render json: foo does not actually do the same thing as foo.to_json does. It does a whole lot of rails magic instead, which is both a lot slower and inconsistent.

Changing this to use page.to_json ensures consistency with the props output in inertia.html.erb, avoiding weird inconsistencies between props renders and full page renders, and is also a lot faster (especially if you are using a library like Oj to optimise json generation in the first place).
@BrandonShar
Copy link
Collaborator

This is awesome, thanks @alexspeller !

Out of curiosity did you do any profiling or metrics for this? It would be cool to see the difference. No need to create something if you didn't.

@alexspeller
Copy link
Author

I made a controller that renders this data structure (from memory, so not including the time to generate it):

    data = (1..1_000_000).map {
      {
        a: rand,
        b: rand,
        c: rand,
        d: rand,
        h: {
          a: rand,
          b: rand,
          h: {
            a: rand,
            b: rand,
          },
        },
      }
    }

With Oj.optimize_rails

Initia page load rendering html: 2655ms
Inertia json render without to_json: 6150ms
Inertia json render with to_json: 2131ms

Without Oj.optimize_rails

Initia page load rendering html: 9634ms
Inertia json render without to_json: 14824ms
Inertia json render with to_json: 8752ms

I believe this gets worse with the complexity of the data structure, as render json: foo calls as_json recursively and also calls object.dup a bunch, so uses a lot more time and memory, whereas my understanding is that to_json calls to_json recursively.

@BrandonShar
Copy link
Collaborator

Wow, that's a big difference! I'm source diving a bit in Rails to understand this better and it looks like this mimics the logic here

Am I looking in the wrong place or is there something happening elsewhere first?

@bknoles
Copy link
Collaborator

bknoles commented Jan 8, 2025

Love this and love the helpful writeup! If I know @skryukov I suspect he did a backflip of joy when he saw "5x faster"!

Any reason not to merge this @BrandonShar ?

@BrandonShar
Copy link
Collaborator

@bknoles Only because I'm genuinely not clear on why it's working based on that link I posted above. It looks like the same logic is happening just in different places so I'm hoping to understand a bit more about what's happening here (or be told I'm looking in the wrong place in rails source, it would not be the first time 😄)

@bknoles
Copy link
Collaborator

bknoles commented Jan 8, 2025

ah ok, i didn't realize that was why you were source diving

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.

3 participants