[GitHub] tomcat pull request #70: Add new accesslog valve pattern %X for recording co...
GitHub user zemian opened a pull request: https://github.com/apache/tomcat/pull/70 Add new accesslog valve pattern %X for recording connection status - https://bz.apache.org/bugzilla/show_bug.cgi?id=61164 You can merge this pull request into a Git repository by running: $ git pull https://github.com/zemian/tomcat zemian_61164-accesslog Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tomcat/pull/70.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #70 commit 63e749b4d57c9d38ce2b6dc3e8fd54f200f60d61 Author: Zemian Deng <zemian.d...@bnymellon.com> Date: 2017-07-21T03:10:53Z Add new accesslog valve pattern %X for recording connection status - https://bz.apache.org/bugzilla/show_bug.cgi?id=61164 --- 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 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 pull request #72: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=...
GitHub user zemian opened a pull request: https://github.com/apache/tomcat/pull/72 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=59901 Caching jsp file read to improve performance Hi, I added jsp file content cache inside ParserController for this enhancement. This reduced two read I found during the parsing for both XML jsp and standard jsp. Impl notes: The ParserController instance seems to be bounded to JspServletWrapper lifecycle though, so this will cache only per JspServletWrapper instance context as well. I have seen that some tag file processing will create nested instances of JspServletWrapper, and in this case, we still not able to use the cache. I wasn't sure was there a better place to add the cache that can reuse by multiple JspServletWrapper instances at this point. Please let me know what you think what you think so far. You can merge this pull request into a Git repository by running: $ git pull https://github.com/zemian/tomcat zemian_59901-jsp-file-cache Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tomcat/pull/72.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #72 commit 9dff7cf58645eb0e2f2b08333b225373d9144361 Author: Zemian Deng <zemian.d...@bnymellon.com> Date: 2017-08-06T21:09:07Z Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=59901 Caching jsp file read to improve performance --- 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 pull request #106: Fix cygpath is checking for empty path
GitHub user zemian opened a pull request: https://github.com/apache/tomcat/pull/106 Fix cygpath is checking for empty path Hi, When running Tomcat under Cygwin shell, I see `cygpath: can't convert empty path` error message printed, but it will continue to work. Looking closely, it seems that `catalina.sh` script didn't check for empty string for the optional JAVA_ENDORSED_DIRS var before calling cygpath. I have created a PR that fix this minor bug. Please review. ``` $ bin/catalina.sh version cygpath: can't convert empty path Using CATALINA_BASE: C:\Users\zemian\apps\apache-tomcat-8.5.29 Using CATALINA_HOME: C:\Users\zemian\apps\apache-tomcat-8.5.29 Using CATALINA_TMPDIR: C:\Users\zemian\apps\apache-tomcat-8.5.29\temp Using JRE_HOME:C:\Users\zemian\apps\jdk-8u161 Using CLASSPATH: C:\Users\zemian\apps\apache-tomcat-8.5.29\bin\bootstrap.jar;C:\Users\zemian\apps\apache-tomcat-8.5.29\bin\tomcat-juli.jar Server version: Apache Tomcat/8.5.29 Server built: Mar 5 2018 13:11:12 UTC Server number: 8.5.29.0 OS Name:Windows 7 OS Version: 6.1 Architecture: amd64 JVM Version:1.8.0_161-b12 JVM Vendor: Oracle Corporation ``` You can merge this pull request into a Git repository by running: $ git pull https://github.com/zemian/tomcat zemian_fix-cygpath Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tomcat/pull/106.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #106 commit 2534d5c3e6359e700690168998d8bd25c273b9b1 Author: Zemian Deng <zemian.deng@...> Date: 2018-04-04T14:43:02Z Fix cygpath is checking for empty path --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
[GitHub] tomcat pull request #132: Fix cygpath error by adding cond check with JAVA_E...
GitHub user zemian opened a pull request: https://github.com/apache/tomcat/pull/132 Fix cygpath error by adding cond check with JAVA_ENDORSED_DIRS The new `bin/tool-wrapper.sh` script gives the error `cygpath: can't convert empty path` when used in Cygwin shell. This is due to `JAVA_ENDORSED_DIRS` was converted even when it's empty. This PR fixes the error by checking if `JAVA_ENDORSED_DIRS` is not empty first. You can merge this pull request into a Git repository by running: $ git pull https://github.com/zemian/tomcat zemian-cygpath-error Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tomcat/pull/132.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #132 commit 7d71bb72e64ac5a9e167f2ad71eb44a7183e2d1a Author: Zemian Deng Date: 2018-11-11T00:15:50Z Fix cygpath error by adding cond check with JAVA_ENDORSED_DIRS --- - To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org