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

[5.x] Reduce the number of times the fieldsCache is reset #9585

Merged
merged 9 commits into from
Apr 16, 2024
Merged

[5.x] Reduce the number of times the fieldsCache is reset #9585

merged 9 commits into from
Apr 16, 2024

Conversation

JohnathonKoster
Copy link
Contributor

@JohnathonKoster JohnathonKoster commented Feb 24, 2024

This PR reduces number of times the fieldsCache is reset inside the Blueprint class. If the new parent has the same blueprint has the previous parent, it will continue to reuse those fields instead of clearing the cache every time.

Note: The following logic (https://github.com/statamic/cms/pull/9585/files#diff-00e780e9caacd1aa9dcb1834686226b1818cc7c99fc24445a2057d09df87e755R629) has been successful in reducing the number of times the fieldsCache is reset, but wouldn't mind this being sanity-checked 🙂

@JohnathonKoster JohnathonKoster marked this pull request as draft February 25, 2024 17:12
@JohnathonKoster JohnathonKoster marked this pull request as ready for review February 25, 2024 20:30
@JohnathonKoster JohnathonKoster mentioned this pull request Feb 25, 2024
37 tasks
Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please target master and adjust the PR title to show [5.x].

@JohnathonKoster JohnathonKoster changed the base branch from 4.x to master March 8, 2024 01:08
@JohnathonKoster JohnathonKoster changed the title [4.x] Reduce the number of times the fieldsCache is reset [5.x] Reduce the number of times the fieldsCache is reset Mar 8, 2024
@jasonvarga jasonvarga dismissed their stale review March 11, 2024 16:55

Requested changes were made.

Copy link
Member

@jasonvarga jasonvarga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The /references page of the docs will reset 410 times and with this PR it drops down to 153. The home page goes from 96 to 21.

I don't see any noticeable difference in load times, but this PR is only a handful of lines now so I don't think it hurts.

@jasonvarga jasonvarga merged commit f8f0226 into statamic:master Apr 16, 2024
16 checks passed
@jasonvarga jasonvarga deleted the call-reset-field-cache-less branch April 16, 2024 14:01
@aerni
Copy link
Contributor

aerni commented May 10, 2024

This PR causes some major issues when extending blueprints in my Advanced SEO addon. I haven't quite figured out at what point the caching is causing issues, but removing the lines of this PR fixes it. I'm not sure what's the best way to move forward?

jasonvarga pushed a commit that referenced this pull request May 20, 2024
Co-authored-by: John Koster <john@stillat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants