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

Refactor and improvements to HttpContent Factories #8767

Merged
merged 44 commits into from
Nov 16, 2022
Merged

Refactor and improvements to HttpContent Factories #8767

merged 44 commits into from
Nov 16, 2022

Conversation

lachlan-roberts
Copy link
Contributor

  • flatten cache in HttpCachingHttpContentFactory
  • remove old CachedContentFactory
  • HttpContent uses RetainableByteBuffer
  • use buffer pool in CachingHttpContentFactory
  • add configuration to reduce/prevent file system accesses for cached HttpContent
  • add separate HttpContent factory for file mapping

@lachlan-roberts
Refactor for CachingContentFactory and ResourceService
1353f16 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
only check file modified time once per second for cached files
a7383ea 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
fix stampede creating CachingHttpContent & remove usage of getPreComp…
6a6681a 
…ressedContentFormats

Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
remove usage of HttpContent.getPreCompressedContentFormats for jetty-…
9f86212 
…core & ee10

Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
remove getPreCompressedContentFormats from HttpContent
81f67f3 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
remove getPrecompressedContents from HttpContent and ee9 impl
6db4bf8 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
use jetty-core ResourceContentFactory in ee9
affa747 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
replace use of CachedContentFactory with CachingContentFactory
6771db4 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
cleanups of HttpContentWrapper and ResourceService
2d161bf 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
extract the generation of getCompressedContentFormats to separate con…
57f71ce 
…tentFactory

Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
cache more fields in CachingHttpContent
64a5b07 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
work in progress to improve CachingContentFactory
e910a8e 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
further work on CachingContentFactory
fbceb92 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
Merge remote-tracking branch 'origin/jetty-12.0.x' into jetty-12.0.x-…
cccc16c 
…HttpContent-Caching-Refactor
@lachlan-roberts
fix failing test cases for ResourceService and Etags
bbd7ace 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
Rename all HttpContent factories
ad7ae3c 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
cleanups of HttpContent factories
3c08931 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
improve configuration options for HttpContent.Factory
86da43a 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
separate evicting logic from CachingHttpContentFactory
8768725 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
use RetainableByteBuffer for getBuffer in HttpContent
0e1bae4 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
fix bugs in CachingHttpContentFactory, ResourceService and FileMapped…
98c6675 
…HttpContentFactory

Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
ResourceHandlerTest should validate cache entry on every request
964108f 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
cannot add ContentFactory to servletContext attribute as there may be…
02e0a39 
… multiple factories in a single context

Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
Merge remote-tracking branch 'origin/jetty-12.0.x' into jetty-12.0.x-…
7752720 
…HttpContent-Caching-Refactor
@lachlan-roberts
use name annotation for EvictingCachingContentFactory constructors
af19607 
Signed-off-by: Lachlan Roberts <[email protected]>
lorban
lorban requested changes Nov 7, 2022
View reviewed changes
jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/FileMappedHttpContentFactory.java Outdated Show resolved Hide resolved
jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/FileMappedHttpContentFactory.java Outdated Show resolved Hide resolved
jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/RetainableByteBuffer.java Outdated Show resolved Hide resolved
jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/TryPathsHandlerTest.java Show resolved Hide resolved
jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java Show resolved Hide resolved
jetty-ee9/jetty-ee9-servlet/src/main/java/org/eclipse/jetty/ee9/servlet/DefaultServlet.java Show resolved Hide resolved
jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingHttpContentFactory.java Outdated Show resolved Hide resolved
jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingHttpContentFactory.java Outdated Show resolved Hide resolved
jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingHttpContentFactory.java Outdated Show resolved Hide resolved
jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingHttpContentFactory.java Outdated Show resolved Hide resolved
sbordet
sbordet requested changes Nov 8, 2022
View reviewed changes
jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/FileMappedHttpContentFactory.java Outdated Show resolved Hide resolved
jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/FileMappedHttpContentFactory.java Outdated Show resolved Hide resolved
jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpContentWrapper.java Outdated Show resolved Hide resolved
jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/FileMappedHttpContentFactory.java Outdated Show resolved Hide resolved
jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/FileMappedHttpContentFactory.java Outdated Show resolved Hide resolved
jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java Outdated Show resolved Hide resolved
jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java Outdated Show resolved Hide resolved
jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/TryPathsHandlerTest.java Show resolved Hide resolved
jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceHandler.java Show resolved Hide resolved
jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/ValidatingCachingContentFactory.java Outdated Show resolved Hide resolved
@gregw gregw dismissed their stale review November 8, 2022 10:51

switching to a non blocking review. I just have some questions now.

sbordet
sbordet requested changes Nov 9, 2022
View reviewed changes
jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CachingHttpContentFactory.java Outdated Show resolved Hide resolved
jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpContent.java Outdated Show resolved Hide resolved
jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/ResourceHandler.java Outdated Show resolved Hide resolved
jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java Show resolved Hide resolved
jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/ResourceHandlerTest.java Outdated Show resolved Hide resolved
jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/TryPathsHandlerTest.java Show resolved Hide resolved
jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/handler/TryPathsHandlerTest.java Show resolved Hide resolved
jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/ResourceHandler.java Outdated Show resolved Hide resolved
@lachlan-roberts
fixes to the CachingHttpContentFactory caching algorithm
387d380 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
add return to BufferUtil.readFrom javadoc
24ce284 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
changes from review
1c6d36e 
Signed-off-by: Lachlan Roberts <[email protected]>
@lachlan-roberts
Merge remote-tracking branch 'origin/jetty-12.0.x' into jetty-12.0.x-…
eeb4df9 
…HttpContent-Caching-Refactor
@lachlan-roberts
remove setter for HttpContentFactory on ResourceHandler
1266581 
revert to using computeIfAbsent on CachingHttpContentFactory
make direct buffers configurable on CachingHttpContentFactory

Signed-off-by: Lachlan Roberts <[email protected]>
@gregw
Copy link
Contributor

gregw commented Nov 15, 2022

@lachlan-roberts @sbordet @lorban can we all work together to land this one. It has been outstanding too long.

gregw
gregw approved these changes Nov 15, 2022
View reviewed changes
Copy link
Contributor

@gregw gregw left a comment

Choose a reason for hiding this comment

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

I'm OK for t his to be merged as an interim step.

@gregw gregw mentioned this pull request Nov 15, 2022
Closed
@lachlan-roberts lachlan-roberts merged commit d72e397 into jetty-12.0.x Nov 16, 2022
@lachlan-roberts lachlan-roberts deleted the jetty-12.0.x-HttpContent-Caching-Refactor branch November 16, 2022 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregw gregw gregw approved these changes

@joakime joakime Awaiting requested review from joakime

@sbordet sbordet Awaiting requested review from sbordet

@lorban lorban Awaiting requested review from lorban

Assignees

@lachlan-roberts lachlan-roberts

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lachlan-roberts @gregw @joakime @sbordet @lorban