On Thursday, March 10, 2016 10:44 PM, ma...@apache.org wrote:
> We'll have to agree to disagree on that one. If you are concerned
> about a performance issue then you need to know where to look to
> enable debug logging. A profiler will tell you where to look and
> at that point you don't need the debug logging.

Well, first of all, I found this without knowing where to look. I just checked 
the thread dump, looked closer at the stack traces involving the 
ClassNotFoundExceptions, and found the try catch clause. If there would have 
been a log.debug(...) or log.trace(...) statement there, then I would know how 
to enable debug logging for this. All without knowing beforehand where to look, 
and all without anybody telling me where to look.

Secondly, not all problems are easily reproducible in a 
development/test/staging environment, especially if one depends on the nature 
of the production traffic to find all (or at least most) places in the code 
that experience problems. Not all organizations have the time and/or the money 
to replay/simulate live traffic in another environment. And I suspect that most 
IT departments and/or service providers would react with some suspicion or 
worry if one would try and get them to introduce a profiler in production, even 
though these tools have become more and more production friendly over the years.

Thirdly, even if profiling would be an option, if no profiling is setup already 
then it would take some time and money to do that. Compared to the trivial 
change of logging level for a specific class. And I can't really see how 
log.debug(...) or log.trace(...)  (possibly surrounded by a 
log.isDebugEnabled() or isTraceEnabled())would have any negative effect for 

Just my two cents...

> The code in question is in the JSP API JAR and there is no 
> configuration API available. The only option would be 
> a system property which would make it a global configuration option.

I'm not sure I understand why the absence of a "configuration API" would stop 
you from simply checking for a specific init parameter in the servlet context 
(thus making it possible to configure it in the web.xml using a context-param). 
And when it comes to per jsp configuration, one could check for a page scoped 
attribute with the same name. Ideally this could be added as a new page 
directive attribute (like <%@ page disableELImports="true" %>), but I guess 
that would constitute a "configuration API" change that would have to be in the 
specification. But the init parameter or page scoped attribute could be fine 
interim solutions, until the specification is updated to add these 
configuration options the proper way.

> Um, no. See above. This would have to be a global option. It would work for 
> some users/pages but not for others.

Um, yes, see above. ;)

And why would it not work for some users? I'm not saying it would magically 
solve the problem for everyone. But it would solve the worst problems for most 
people, and it wouldn't make it worse for anyone (ie, the fact that one would 
have the *option* to turn it off, doesn't make the situation worse).

> Generally, the suggestions are reasonable. The control can't be as 
> fine-grained as you suggest but that isn't necessarily a show-stopper.

Well, the solution with the page scoped attribute in the jsp page would make it 
fine grained enough for most people, don't you think? :)

I would also be interested in hearing your thoughts about having an option to 
disable this class lookup for all names that doesn't start with a capital 
letter. Thus making ${Boolean.TRUE} work as normal, while skipping ${foo.bar}, 
even if they coexist in the same jsp page. I actually would think that would be 
the best solution to this, since the naming convention dictates that you should 
start class names with capital letter, and variables with lower case letters.

Basically, the code could look something like this, in 

if (importHandler != null) {
        if (resolveClass) {
                //disableELImportsForNamesNotStartingWithUpperCase is false by 
                //and can be configured using for example a web.xml init 
                if (disableELImportsForNamesNotStartingWithUpperCase && 
!Character.isUpperCase(key.charAt(0))) {
                        resolveClass = false;
        if (resolveClass) {

> The main disadvantage that these options have is that a 
> better solution is available. Follow the bug report from 
> my previous message in this thread for details.
> [...]
> In this case, I believe the root cause has been fixed.

That sounds great! And I saw now in the bug report page that your simple test 
case indicated a ~1ms performance gain per avoided ClassNotFoundException. That 
sounds very close to my own tests, where the start page takes about 2500 ms, 
and I counted about 2700 ClassNotFoundExceptions.

But, just a question. When looking at the code change in commit 1734419, it 
seems to only handle the special case of ${foo} or ${empty foo} and similar. 
But the problem would still remain for ${foo.bar}, right? So, technically, the 
root cause is not fixed completely. We have lots of ${foo.bar} equivalents in 
our code, where it is not uncommon for "foo" to be null. So I suspect that we 
still would experience a big performance degradation on our sites, even with 
this fix, compared to simply using Tomcat 7.


P.S. Having said all this. I hope you don't think I sound like an overcritical 
fault-finding bastard. I just find it interesting do discuss this. :) And a 
solution to this problem would mean that we didn't have to revert back to 
Tomcat 7 for our sites. D.S.

To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org

Reply via email to