Skip to content

Slowness after upgrading to 3.9 #295

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

Closed
integralhero opened this issue Jan 24, 2022 · 8 comments · Fixed by #296
Closed

Slowness after upgrading to 3.9 #295

integralhero opened this issue Jan 24, 2022 · 8 comments · Fixed by #296
Assignees

Comments

@integralhero
Copy link

Hey there, we recently bumped to 3.9 and noticed latency spike (~double) along a critical code-path. We have json schema validation disabled here for performance reasons.

As a comparison, here's a flamegraph I made where our code uses

Release version 3.8 of the ruby-sdk gem:
Screen Shot 2022-01-24 at 10 50 32 AM

vs. 3.9.
Screen Shot 2022-01-24 at 10 50 43 AM

I see a lot of cycles spent in Optimizely::StaticProfileManager, possibly tied to this change

I was wondering if anyone's noticed something similar/ could give me more context on the change here and what we might be able to do to address the performance issues

@msohailhussain
Copy link
Contributor

Can you please share flamegraph SVGs of both releases and it will be helpful if you send diff as well using difffolded.pl

@integralhero
Copy link
Author

Here's a zip containing flamegraphs for 3.8, 3.91, and 3.10

flamegraph.zip

zashraf1985 added a commit that referenced this issue Jan 28, 2022
…SDK initialization (#296)

## Summary
`OptimizelyConfig` object is generated when SDK is being initialized. This operation is heavy and blocks the SDK from initializing quickly. This object is only used when users explicitly access it using `get_optimizely_config` API call. This PR moves the creation of this object from SDK initialization to the actual API call.

## Test plan
- Manually tested thoroughly
- All unit tests pass
- All FSC tests pass

## Issues
fixes #295
@Danny-Driscoll
Copy link

@integralhero Re-opening after the auto-close from the PR merge. We just merged a change that we believe will improve performance on the initialization process for the SDK by deferring some data processing until it is explicitly accessed vs. running it on initialization. The data processing wasn't entirely new in 3.8 but we did expand the scope of data being processed in that version, so we suspect it would help address a change in that version. It's on Master now if you are able to validate off that whether you see an improvement in your test setup.

@integralhero
Copy link
Author

Thanks so much for following up @Danny-Driscoll! Just tried it out and it looks fine. Cheers!

@integralhero
Copy link
Author

integralhero commented Feb 1, 2022

Could we incorporate this in the next minor version bump? (not sure how y'all do releases)

@Danny-Driscoll
Copy link

Yep, this will be released shortly in a new minor version. We just wanted to get some validation that it did indeed improve your observed performance before pushing it out.

@zashraf1985
Copy link
Contributor

@integralhero
Copy link
Author

Thanks! Closing this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants