[GitHub] tomcat issue #72: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=59901
Github user zemian commented on the issue: https://github.com/apache/tomcat/pull/72 Okay Mark. At this point, I am not sure I have a full grasp of the solution you wanted yet. I will study little more and revisit this issue later. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #72: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=59901
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/72 My point is that with the o.a.c.webresources.Cache in place, this solution offers little additional benefit. I'm not suggesting creating dependencies from Jasper to Catalina. On closer inspection, caching the content in the ParserController does limit the lifetime of the cache appropriately. There are more than 2 reads per file. The process to determine encoding is another read. My thinking is really just a different solution. It just caches the content in a different place - a buffered input stream. From memory there was also the opportunity to reduce some unnecessary processing (the result would be unchanged from the #parseDirectives phase) in the #parse phase. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #72: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=59901
Github user zemian commented on the issue: https://github.com/apache/tomcat/pull/72 Hi Mark, > Tomcat provides (by default) a static content cache in the resources layer so I do wonder if there is much benefit in caching the content in the JSP engine as well. Are you referring to `org.apache.catalina.webresources.Cache`? To me this enhancement is at jasper level, so I thought it make more sense to keep it self contained. Also, is okay to bring catalina classes into jasper package? But any rate, if you do decide we go this route, please confirm and I will look into it more. > The proposed cache appears to be unbounded which is likely to be a problem for larger installations. Also, there is no need to cache the JSP content once it has been processed. I thought about this, but my current proposed cache is already bounded by `ParserController` instance, and it is already per request only. You still think we should limit cache further? If we do this, then we add much more complexity to caching though. Please confirm if we need this. > My original thinking was more along the lines of some refactoring to allow re-use of a single buffered input stream with some limit on the buffer size that could mean the stream did need to be re-opened for large JSPs. I am not clear on this suggestion yet. Do tell me more if you strongly feel that's more the right direction. My current proposed solution is to cache the jsp file content as char array, then re-feed into different Stream/Reader that is used by jasper parser. The previous code will perform two reads per single jsp file processing: once for `ParserController#parseDirectives` and another for `JspDocumentParser#parse`. The proposed code will reduce these to one file read only. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat issue #72: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=59901
Github user markt-asf commented on the issue: https://github.com/apache/tomcat/pull/72 Tomcat provides (by default) a static content cache in the resources layer so I do wonder if there is much benefit in caching the content in the JSP engine as well. The proposed cache appears to be unbounded which is likely to be a problem for larger installations. Also, there is no need to cache the JSP content once it has been processed. My original thinking was more along the lines of some refactoring to allow re-use of a single buffered input stream with some limit on the buffer size that could mean the stream did need to be re-opened for large JSPs. It may be that the static content cache provided in the resources layer provides sufficient benefit that tidying up the multiple input streams provides minimal additional benefit in most cases. Where I would expect it to help is the case when (for whatever reason) caching in the resource layer is disabled. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
