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.

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

Jump to bottom

Weighted proposal voting causes slowness on proposals index page #350

Open
Quentinchampenois opened this issue Nov 8, 2024 · 7 comments · May be fixed by #358
Open

Weighted proposal voting causes slowness on proposals index page #350

Quentinchampenois opened this issue Nov 8, 2024 · 7 comments · May be fixed by #358

Comments

@Quentinchampenois
Copy link
Contributor

Describe the bug
A clear and concise description of what the bug is.

Hello, I share with you slowness we had on a production instance. The feature Weighted proposal voting appears to really slow down the loading of the proposals index page.

I haven't investigate a lot but I think there is a query n+1 in the proposal_m_cell_override or caching issue maybe.

About the application :

  • Decidim v0.27.5
  • DecidimAwesome: Branch release/0.27-stable and commit c72e7f0fa0fe98807ace8414ecc08cbf44ce72d5
  • 600 proposals and 600 ProposalExtraField in database

To Reproduce
Steps to reproduce the behavior:

  1. Enable caching mode
  2. Enable Private fields
  3. Populate database with proposals and private fields
  4. See loading slower

Expected behavior
A clear and concise description of what you expected to happen.

I expect the loading page to remain stable

Quick fix

Deactivate the feature weighted_proposal_voting if not used 👍

Example: config/initializers/decidim_awesome.rb

Decidim::DecidimAwesome.configure do |config|
  config.weighted_proposal_voting = :disabled
end

Example of rendering time when activating and disabling the feature :

--------- With the weighted_proposal_voting enabled
[5400a7ce-4888-4cb3-97a0-0679b1a57269] [127.0.0.1] Completed 200 OK in 5965ms (Views: 5310.3ms | ActiveRecord: 578.5ms | Allocations: 5574239)
[0188884e-70bf-4388-9042-6464a93d3011] [127.0.0.1] Completed 200 OK in 4866ms (Views: 3645.9ms | ActiveRecord: 979.1ms | Allocations: 2583164)
[a83a965d-fe61-4246-88b2-9d116f3ac3f0] [127.0.0.1] Completed 200 OK in 2407ms (Views: 2221.7ms | ActiveRecord: 127.0ms | Allocations: 1004768)
--------- With the weighted_proposal_voting disabled
[d2f56eed-5ccb-4d4c-8c2b-5e77f7296b3a] [127.0.0.1] Completed 200 OK in 2791ms (Views: 2460.7ms | ActiveRecord: 256.8ms | Allocations: 3737530)
[3db5737a-2cc8-489f-bac0-c014f13dfaad] [127.0.0.1] Completed 200 OK in 734ms (Views: 580.0ms | ActiveRecord: 96.6ms | Allocations: 698351)
[b70c1b26-6ce0-4dbd-beaa-b83bf98f9fb1] [127.0.0.1] Completed 200 OK in 629ms (Views: 455.4ms | ActiveRecord: 85.7ms | Allocations: 697724)
@microstudi
Copy link
Contributor

I've been investigating a bit this and I couldn't replicate this degradation of performance. I've tested thge 0.28 version though.

On the other hand, I don't see how this method generates a n+1 query, in any case just one more query per proposal (which effect is mostly negligible considering the amount of other queries around).

But please, add more information if you can.

Maybe this is relevant decidim/decidim#13701

@microstudi microstudi linked a pull request Dec 2, 2024 that will close this issue
Draft
2 tasks
@microstudi microstudi linked a pull request Dec 3, 2024 that will close this issue
Optimize performance #358
Draft
2 tasks
@microstudi
Copy link
Contributor

@Quentinchampenois
I've made a PR to improve the performance in Awesome that should affect your case.

The backport for the version 0.27 is here:
This is the PR #359

You thing you can try it in your environment? see if there's any gain?

@Quentinchampenois
Copy link
Contributor Author

Thank you @microstudi for your time and the investigation !

I will try to run your optimization on my side to see differences 👍

@Quentinchampenois
Copy link
Contributor Author

Quentinchampenois commented Dec 9, 2024 •
edited
Loading

Hey I tested locally your optimizations, here is what I got on v0.27 with 700 proposals and cache enabled :

With a simple Gatling scenario and a ramp-up of 10 users in 20 seconds, executed 6 times, here is my observations:

  • With original configuration and weighted_proposal_voting activated, 50th percentile is : 16915 ms
  • With your optimization and weighted_proposal_voting activated, 50th percentile is : 14471 ms. Your optimization enhances around 14% of response time
  • With your optimization and weighted_proposal_voting activated, and also with changes you made on app/cells/concerns/decidim/decidim_awesome/proposal_l_cell_override.rb backported on app/cells/concerns/decidim/decidim_awesome/proposal_m_cell_override.rb, 50th percentile is : 1390 ms. Main reason is because there is no more the model.extra_fields&.reload in cache_hash method which reloads the resource on index page where 20 proposal are loaded (then at least 40 requests)

To me your optimizations improve performances ! Applying changes made from proposal_l_cell_override.rb to proposal_m_cell_override.rb is even better

@microstudi
Copy link
Contributor

You're right @Quentinchampenois I forgot to change the cell to the M type in the backport. I've applied it.

This is one more optimization that can be done, but it is a little bit tricky. Is the one applied in this commit: f410f39

with this optimization, we memoize at the application level all the queries related to extra_fields and votes. This should reduce the queries from 3 per proposal displayed to just one and 2 extra grouped select for the memoization.

If you can update your test with the latest commit and tell me the performance difference, we will see if it's worth it.

@Quentinchampenois
Copy link
Contributor Author

Hello, I just tested with your latest changes and I can confirm that the 50th percentile is 2020ms on 6 runs (for 2345ms before these changes) 👍

@microstudi
Copy link
Contributor

Thanks @Quentinchampenois unless you thing about any other optimization that could be done, I'll finish the specs and merge this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Optimize performance decidim-ice/decidim-module-decidim_awesome
2 participants
@microstudi @Quentinchampenois