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

Implement a cache in AbstractTearOff even when MetaClass.NO_CACHE is set #184

Merged
merged 1 commit into from
Jun 10, 2020

Conversation

jglick
Copy link
Member

@jglick jglick commented Apr 30, 2020

Implements my proposal from jenkinsci/maven-hpi-plugin#40 (comment). Jenkins in development mode—hpi:run, jetty:run, surefire:test—sets MetaClass.NO_CACHE flag. (Usually via the system property stapler.jelly.noCache, though in fact it affects Groovy views too.) This flag ensures that changes to source files are applied on every new page load, which is good, but also forces every *.jelly or *.groovy file to be reparsed whenever accessed—in some cases multiple times in a single HTTP response! That adds a lot of CPU overhead and contributes to page loads being noticeably slower than in production.

This patch retains most of the NO_CACHE behavior (it is interpreted in a number of places), but caches parsed/compiled views if they have not actually been changed on disk. There is still a bit of overhead to check file timestamps but things feel quite a bit snappier during hpi:run, especially on a form-heavy page like /configure.

@jglick jglick requested a review from daniel-beck April 30, 2020 19:09
@jglick jglick marked this pull request as draft April 30, 2020 19:10
@jglick jglick force-pushed the AbstractTearOff-cache branch from 94841fd to 1555fd1 Compare April 30, 2020 19:11
@jglick jglick marked this pull request as ready for review April 30, 2020 19:11
@jglick
Copy link
Member Author

jglick commented Jun 10, 2020

or @jvz @jeffret-b @Wadeck

Copy link
Contributor

@jeffret-b jeffret-b left a comment

Choose a reason for hiding this comment

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

Looks good. It should perform handy caching for those development situations, to speed things up nicely.

It's a reasonable adjustment to the NO_CACHE behavior. It still meets the necessary behavior of that flag, but provides a simple, but helpful cache to speed things up.

@jglick jglick merged commit 70d5b2f into jenkinsci:master Jun 10, 2020
@jglick jglick deleted the AbstractTearOff-cache branch June 10, 2020 21:05
timja added a commit to timja/stapler that referenced this pull request Oct 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants