[GitHub] [tomcat] kamnani commented on pull request #352: Optimizing Resource Lookup using Bloom Filter
kamnani commented on pull request #352: URL: https://github.com/apache/tomcat/pull/352#issuecomment-697550277 > I tried it, with some changes. Will see the feedback about an actual performance benefit. I measured the cost of building the filter, not too big overall so there's likely something. Thanks for your time on this @rmaucher This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on pull request #352: Optimizing Resource Lookup using Bloom Filter
kamnani commented on pull request #352: URL: https://github.com/apache/tomcat/pull/352#issuecomment-696834567 This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on pull request #352: Optimizing Resource Lookup using Bloom Filter
kamnani commented on pull request #352:
URL: https://github.com/apache/tomcat/pull/352#issuecomment-696930075
> There's no cache here: the AbstractArchiveResourceSet already has a
"JarFile archive" field, so it adds a new JarContents field with the same data
and both get created and cleared at the same time. Looking back at the other
#340 there were problems with the loader/classloader modifications which IMO
look like a big hack and have a JAR cache.
I think, your solution does make sense to me. I will still need these code
lines in AbstactArchiveResourceSet just to skip checking a jar if the resource
is not going to be found there.
```
if (!jarContents.mightContainResource(path, webAppMount)) {
return new EmptyResource(root, path);
}
```
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on pull request #352: Optimizing Resource Lookup using Bloom Filter
kamnani commented on pull request #352:
URL: https://github.com/apache/tomcat/pull/352#issuecomment-696834567
> I made the change to use the Context instead of the Host, no issue there.
> However, I'm not ok with the proposed changes as is. The problem for me is
that I don't understand the weird code added to AbstractArchiveResourceSet. I
am ok with adding the creation of that JarContents filter in openJarFile()
[even with the sync ...], basically, but not elsewhere, and without the strange
custom refresh mechanism on lookup.
> Then, since it is _likely_ quite harmless, it could be enabled by default
(and maybe the flag can be removed altogether ...).
@rmaucher I agree for the creation of JarContent in OpenJarFile(), however
the crux of the optimization will be when we are checking for the resource in a
Jar. So we still need to add this code inside getResource() call, the way its
been in the PR after we create the JarContent.
```
if (!jarContents.mightContainResource(path, webAppMount)) {
return new EmptyResource(root, path);
}
```
For the custom refresh mechanism, we added that with regard to PR #340
where Mark(comment attached below) had asked us about the cache validation
mechanism.
```
I don't see any cache validation meaning updated JARs (with the same name)
will be ignored. That is likely to trigger all sorts of unexpected bugs.
```
I would expect OpenJarFile already must be tackling the jar-refresh issue
and in that scenario your comment would be the best way to go forward with
this.
Thanks again for your valuable time on this. I look forward for this
optimization to get shipped soon :)
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on pull request #352: Optimizing Resource Lookup using Bloom Filter
kamnani commented on pull request #352: URL: https://github.com/apache/tomcat/pull/352#issuecomment-694947975 @rmaucher Thanks for the comment. We tested this optimization on an application with 3500+ jars which performed 60,000+ class loads. This sliced off 180+ seconds from the startup time. I am sure this is a really good optimizations for large applications. I can add those flags to the Context as you did with the other PR. This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on pull request #352: Optimizing Resource Lookup using Bloom Filter
kamnani commented on pull request #352: URL: https://github.com/apache/tomcat/pull/352#issuecomment-694940080 @martin-g Thanks for your valuable feedback. I have added the test cases and marked the conversations as resolved after the latest commits. Do we have anything else on this PR? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] [tomcat] kamnani commented on pull request #352: Optimizing Resource Lookup using Bloom Filter
kamnani commented on pull request #352: URL: https://github.com/apache/tomcat/pull/352#issuecomment-689825942 @martin-g Thanks for your time on this : ) I have marked all of the conversations as resolved after the latest commits. Do we have any more feedback on this PR? This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
