Reduce amount of results in dropdowns #818
Conversation
Fixes #816 By reducing the amount of results by default (100), and narrowing results based on year input, the memory usage on the page is dramatically reduced.
| @@ -47,7 +47,14 @@ | |||
|
|
|||
| <div class="field_wordcamp-id"> | |||
| <label for="wordcamp-id">WordCamp <span>(optional)</span></label> | |||
| <?php echo get_wordcamp_dropdown( 'wordcamp-id', array(), $wordcamp_id ); ?> | |||
| <?php if ( ! isset( $_GET[ 'action' ] ) ) { | |||
| // Show only latest 100 WordCamps by default | |||
There was a problem hiding this comment.
Pre-COVID, we had around 150 camps per year, so I'd lean towards something more like 300 or 400, which should still be totally fine for memory.
We only recently started noticing the memory warnings, and it's not running out of memory yet, so I'm hesitant to make it harder for folks to access the data. I'm guessing we could do something like 800 safely, and it may be better to make the minimal necessary impact.
There was a problem hiding this comment.
only recently started noticing the memory warnings, and it's not running out of memory yet
that's fair, although i'm always a little nervous when it almost reaches the limit, and could be knocked over by another ~100 WordCamps :)
There was a problem hiding this comment.
Yeah, that's fair, it'd be good to do something now so that it doesn't get worse 👍🏻
Right now it seems like some of the other approaches are better (reducing the data, caching, fetching async), but if we have to then limiting the # of camps could work.
| // If something is being searched, return WordCamps for that year, or the default latest year, to reduce memory usage | ||
| $wc_year = ( isset( $years ) ) ? $years : date( 'Y' ); | ||
| echo get_wordcamp_dropdown( 'wordcamp-id', array( 'date_query' => array( 'year' => $year, 'compare' => '=' ) ), $wordcamp_id ); | ||
| } ?> |
There was a problem hiding this comment.
So if someone wanted to view WCUS 2018 info, would they have to do this?
- Select
2018 - Select
Entire Year - Press
Show Results - Select the camp from the dropdown now that it's there
- Press
Show Resultsagain
I'm guessing most folks won't realize they need to do that. We might need to add some instructions, or consider a more intuitive flow.
If caching would solve the problem without an UI/UX changes, I think that'd be preferable.
There was a problem hiding this comment.
If #816 (comment) would solve the problem
let me re-look at a caching option and see what that looks like.
I'm unsure what the normal workflow here is (presumably it's in stats somewhere), although it does suggest that the dropdown is optional, and also if you search by a single year, you do get different year results - presumably based on when the site was created?
There was a problem hiding this comment.
👍🏻
If caching doesn't help, another option would be to refactor the flow. instead of populating the dropdown during the initial page load, we could do it async via XHR every time the year or time period changes. Then the XHR could only fetch the camps during that year.
Actually, just fetching the data via XHR without any year filtering might be enough for now, since the data needed to compile the page would be split across two processes. We may eventually reach a point where we can't show all camps, but I'd be surprised if we're anywhere close to that with only 1400 sites.
🤔 That makes me think that reducing the data is worth another look, since it seems like the best option if it works.
| <?php echo get_wordcamp_dropdown( 'wordcamp-id', array(), $wordcamp_id ); ?> | ||
| <?php if ( ! isset( $_GET[ 'action' ] ) ) { | ||
| // Show only latest 100 WordCamps by default | ||
| echo get_wordcamp_dropdown( 'wordcamp-id', array( 'posts_per_page' => 100, 'order' => 'DESC' ), $wordcamp_id ); |
There was a problem hiding this comment.
🤔 Are there other reports that we should make this change to? Why would payment-activity be the only one needed when they all call get_wordcamp_dropdown() with the default $query_options? Ah, searching for in:#dotorg-wordcamp-logs peak memory usage does show others having issues.
Maybe the change should be deeper, in get_wordcamp_dropdown() itself? Or more DRY, like by having all the wordcamp-reports/ call it with a WordCamp\Reports\Report\Base::get_dropdown_options() argument?
It might also be worth considering reverting e058b4b , so that the logs aren't cluttered with false positives. I enabled that after 95% was cleaned up, just to see if there were others getting close, but 90 may be too low in practice.
There was a problem hiding this comment.
🤔 Are there other reports that we should make this change to?
Yes, I was mainly limiting the impact of any change being suggested here - should have said that in the original issue
|
Never got around to doing much with this, so going to close but keep in mind. |
Fixes #816
By reducing the amount of results by default (100), and narrowing results based on year input, the memory usage on the page is dramatically reduced.