Re: Intermittent ClassNotFoundException in Jasper EL evaluation
On 15/03/2016 17:18, jimi.hulleg...@svensktnaringsliv.se wrote: > On Friday, March 11, 2016 10:03 PM, ma...@apache.org wrote: >> >> Monday works. I might try experimenting with some ideas between now and then >> anyway. > > Hi again, > > A bit shorter reply this time, because of time shortage. I preferred to focus > the little time I managed to "break free" on testing the trunk version of > Tomcat 8 :) > > And... it looks really great! I have performed some JMeter tests now, and > here are the results: > > Tomcat 8.0.32 > Average 2500 ms per request. > > Tomcat 8 trunk (revision 81735040) > Average 360 ms per request > > Tomcat 7.0.68 > Average 290 ms per request > > > Test setup: > - Windows 8 > - JMeter 2.13 > - Tomcat with a custom webapp, on Escenic CMS > - JMeter running with one thread, and a throttle (Constant Throughput Timer) > set to 12 samples per minute (ie 5 seconds between each request) > - Testing the start page (with lots of content) > - Tomcat started fresh, and the webapp visited a few times in the browser as > a warmup > - Waited until CPU fan and CPU load settled down before starting the test > - No other traffic to Tomcat during the test > - No resource hungry program running except Tomcat and JMeter > > So, all in all, I see a huge improvement with our site. Going from 2500 ms to > 360 ms. :) > As you can see, compared to Tomcat 7 it is still an increase with 70 ms, but > for our site that is not a "deal breaker". Excellent. If need be, I think there is scope to improve that further but if the results are good enough for you at this point, that work can wait. Mark - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
RE: Intermittent ClassNotFoundException in Jasper EL evaluation
On Friday, March 11, 2016 10:03 PM, ma...@apache.org wrote: > > Monday works. I might try experimenting with some ideas between now and then > anyway. Hi again, A bit shorter reply this time, because of time shortage. I preferred to focus the little time I managed to "break free" on testing the trunk version of Tomcat 8 :) And... it looks really great! I have performed some JMeter tests now, and here are the results: Tomcat 8.0.32 Average 2500 ms per request. Tomcat 8 trunk (revision 81735040) Average 360 ms per request Tomcat 7.0.68 Average 290 ms per request Test setup: - Windows 8 - JMeter 2.13 - Tomcat with a custom webapp, on Escenic CMS - JMeter running with one thread, and a throttle (Constant Throughput Timer) set to 12 samples per minute (ie 5 seconds between each request) - Testing the start page (with lots of content) - Tomcat started fresh, and the webapp visited a few times in the browser as a warmup - Waited until CPU fan and CPU load settled down before starting the test - No other traffic to Tomcat during the test - No resource hungry program running except Tomcat and JMeter So, all in all, I see a huge improvement with our site. Going from 2500 ms to 360 ms. :) As you can see, compared to Tomcat 7 it is still an increase with 70 ms, but for our site that is not a "deal breaker". Regards /Jimi - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: ***UNCHECKED*** RE: Intermittent ClassNotFoundException in Jasper EL evaluation
On 11/03/2016 19:00, jimi.hulleg...@svensktnaringsliv.se wrote: > On Friday, March 11, 2016 6:07 PM, ma...@apache.org wrote: > I'm wasn't talking about gathering information regarding performance. I was > talking about gathering information about what jsp/tag code and what EL > variable names are responsible for this, and how often it happens. In order > to get a grasp of if it is feasible to solve by fixing the code. For example, > if this log output showed me that 90% of the ClassNotFoundExceptions happen > because of a handful of jsp/tag files, then I would just fix these and see > how much that helped. But if the log output would indicate that this problem > is spread out over too many jsp and tag files, then I wouldn't even try and > fix it in the code, but would instead try some other approach (like disabling > this feature, or reverting back to Tomcat 7). Ah, now I understand your motivation. debug logging would be more helpful than a profiler in that case. The next problem is that we are in a spec class and can't add any dependencies. That forces a hard dependency on java.util.logging which isn't great. What you can do is turn on debug logging for the web application class loader. That should get you most of what you need. >> At the point where you'd need to access the ServletContext to read the >> configuration, the ServletContext isn't available. >> [...] >> Since I wrote the "configuration API" above, I did some more research and >> found the route >> via the ELContext. That is what the implemented solution uses. > > So, just to check if I understood you correctly... You *can* get access to > the ServletContext, right? (Using page.getServletContext()...) No, the ServletContext isn't directly accessible but more digging shows some helpful signs. ScopedAttributeELResolver can access the current JspContext. That has access to the page/request/session/application attributes and can probably be cast to PageContext which has access to the ServletContext. So there is a (slightly contrived) route by which per web application config can be passed without breaking the spec and per page config can be passed with custom code or extending the spec (which as far as the spec owners are concerned is the same thing as breaking the spec). >> 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). > > >> Which is my point. The solution doesn't help those users that need imports >> and >> good performance. Greater granularity in configuration helps to a point but >> there will be users that need imports and good performance on the same page >> and per expression configuration is taking things a bit too far for my taste. > > I'm sorry, but I don't follow you now. Are you saying that since there are > people who wouldn't benefit from this (but not get hurt either), there is no > reason to fix it for those that *would* benefit from it? I'm saying that I prefer to expend effort on a solution that doesn't break the spec and works for everyone than one that only works for some users, probably involves breaking the spec and increases configuration complexity. >> Per page is certainly better than global but I still have concerns. In >> addition to the large pages >> needing imports and performance above, I'm concerned that there is no good >> default for >> an option that disables import support. Disabling imports by default will >> break pages that >> use imports unless the developer adds the extra code. Not disabling them by >> default means >> having to add code to every page with a performance problem. > > Well, I never said that it would solve the problem for everyone. But for > people who don't use any EL imports at all (like everyone migrating from > Tomcat 7), they would benefit from a global *off* option, and then they can > systematically enable this feature slowly, page by page, as they add > functionality that depends on it. Page by page config requires breaking the spec which at this point I'd rather not do by default. > As for people who have pages that need both imports and performance, I still > claim that this fix wouldn't make it any *worse* for them. In fact, I would > claim that it would make it somewhat better, because now they would have an > option to control what pages should have this feature, and if they have > important pages that doesn't use imports then they at least could reduce the > performance problem for those pages. And regarding the pages with both a need > for both imports and performance, well they still have the workaround that > was mentioned in the beginning of this thread, adding a specific scope to all > the EL variables (at least the ones that can be null or not set). >
RE: Intermittent ClassNotFoundException in Jasper EL evaluation
On Friday, March 11, 2016 6:07 PM, ma...@apache.org wrote: > > And a debug log message is unlikely to tell you any more than the thread dump > did. That depends on what is actually being logged. If the class name is printed, then one could immediately figure out the name of the EL variable (like "java.lang.myTempValue", then the EL parameter name is "myTempValue"). And the thread dump only gives me a snapshot, containing 0, 1 or more stacktraces with the ClassNotFoundException. Unless I take a threaddump every millisecond (or even more often), the thread dump method would miss some stacktraces. With logging, it would not miss any. > If the same code keeps appearing in a thread dump it is a fair bet that time > is being spent in that code but you still need performance data - which you > can't get from debug logging or thread dumps - to be certain. I'm wasn't talking about gathering information regarding performance. I was talking about gathering information about what jsp/tag code and what EL variable names are responsible for this, and how often it happens. In order to get a grasp of if it is feasible to solve by fixing the code. For example, if this log output showed me that 90% of the ClassNotFoundExceptions happen because of a handful of jsp/tag files, then I would just fix these and see how much that helped. But if the log output would indicate that this problem is spread out over too many jsp and tag files, then I wouldn't even try and fix it in the code, but would instead try some other approach (like disabling this feature, or reverting back to Tomcat 7). > As for using it in production, you aren't going to be using a profiler unless > you have a problem. > I'd look at the risk (never seen an issue) of using a profiler against the > production system and > the quality of data / time to useful results vs debug logging and the longer > to get results / lower > quality of data and choose the profiler every time. YMMV. Well, I guess you have a point. But the fact is that we don't have any profiler tool installed in production right now (and I suspect that is a quite common scenario for many users in general), and having one installed is not something that is done quickly. Enabling trace or debug logging in a specific class takes less than a minute, giving me much needed information more or less instantly. But yes, if one would have followed common sense, and already had a profiler installed (and have used it enough to know how to get it to output the needed information), then you would be right ;) > At the point where you'd need to access the ServletContext to read the > configuration, the ServletContext isn't available. > [...] > Since I wrote the "configuration API" above, I did some more research and > found the route > via the ELContext. That is what the implemented solution uses. So, just to check if I understood you correctly... You *can* get access to the ServletContext, right? (Using page.getServletContext()...) > It should be possible to hook up your proposed configuration option > to pass data via that as long as the code is careful. OK. Sounds promising. Fingers crossed. :) > 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). > Which is my point. The solution doesn't help those users that need imports > and > good performance. Greater granularity in configuration helps to a point but > there will be users that need imports and good performance on the same page > and per expression configuration is taking things a bit too far for my taste. I'm sorry, but I don't follow you now. Are you saying that since there are people who wouldn't benefit from this (but not get hurt either), there is no reason to fix it for those that *would* benefit from it? > Per page is certainly better than global but I still have concerns. In > addition to the large pages > needing imports and performance above, I'm concerned that there is no good > default for > an option that disables import support. Disabling imports by default will > break pages that > use imports unless the developer adds the extra code. Not disabling them by > default means > having to add code to every page with a performance problem. Well, I never said that it would solve the problem for everyone. But for people who don't use any EL imports at all (like everyone migrating from Tomcat 7), they would benefit from a global *off* option, and then they can systematically enable this feature slowly, page by page, as they add functionality that depends on it. As for people who have pages that need both imports and performance, I still claim that this fix wouldn't make it any *worse* for them. In fact, I would clai
Re: Intermittent ClassNotFoundException in Jasper EL evaluation
On 11/03/2016 14:17, jimi.hulleg...@svensktnaringsliv.se wrote: > 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. And a debug log message is unlikely to tell you any more than the thread dump did. If the same code keeps appearing in a thread dump it is a fair bet that time is being spent in that code but you still need performance data - which you can't get from debug logging or thread dumps - to be certain. > 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. You still need to know what debug logging to turn on and if you are guessing based on thread dumps then you already have everything debug logging is likely to give you. > 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 > anyone. Compared to the cost of the time to debug issues without a profiler, the cost of a profiler is tiny. I use YourKit (because they give me a free copy to use for my ASF work) and, while the price of it has gone up quite a bit since I last looked a few years ago, it still (in my view) offers a very quick return on investment. As for using it in production, you aren't going to be using a profiler unless you have a problem. I'd look at the risk (never seen an issue) of using a profiler against the production system and the quality of data / time to useful results vs debug logging and the longer to get results / lower quality of data and choose the profiler every time. YMMV. >> 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. At the point where you'd need to access the ServletContext to read the configuration, the ServletContext isn't available. The code in question is in a spec JAR and we can't change the public API in any way, nor can we add a dependency to some other component. That tends to leave system properties as the only option. Since I wrote the "configuration API" above, I did some more research and found the route via the ELContext. That is what the implemented solution uses. It should be possible to hook up your proposed configuration option to pass data via that as long as the code is careful. > 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). Which is my point. The solution doesn't help those users that need imports and good performance. Greater granularity in configurati
RE: Intermittent ClassNotFoundException in Jasper EL evaluation
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
anyone.
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
ScopedAttributeELResolver.java:
if (importHandler != null) {
if (resolveClass) {
//disableELImportsForNamesNotStartingWithUpperCase is false by
default,
//and can be configured using for example a web.xml init
parameter
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.
Re: Intermittent ClassNotFoundException in Jasper EL evaluation
On 10/03/2016 22:16, Christopher Schultz wrote: > Mark, > > On 3/10/16 4:43 PM, Mark Thomas wrote: >> On 10/03/2016 21:16, jimi.hulleg...@svensktnaringsliv.se wrote: >>> On Thursday, March 10, 2016 11:20 AM, ma...@apache.org wrote: > 3. Why is the problem not limited to the first request for a > jsp page? Because EL imports may be dynamic so the EL has to be evaluated on execution. >>> >>> I'm not really sure I follow you now. Can you explain what you >>> mean with dynamic imports in this regard? I can't see any >>> mentioning of it in the specs >>> (http://download.oracle.com/otn-pub/jcp/el-3_0-fr-eval-spec/EL3.0.FR. > pdf). > >>> >> There is nothing stopping a JSP author obtaining a reference to >> the ImportHandler and conditionally adding classes to import. The >> configuration of the ImportHandler could change on every call to >> the page. > > What about marking the ImportHandler as "dirty" and flushing a cache > of prior lookups? (Or are we talking about spec-defined classes only, > here?) We are talking about spec defined classes here. Also, ELContext and ImportHandler are not cached across requests and a class level cache doesn't work because each page can have different imports. I looked at caching when this issue first arose and we already cache everything we can. Fixing the performance issue required a different approach based on making the affected Resolver more aware of the context in which it was being used so it could skip the ImportHandler in the affected cases. Mark - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Intermittent ClassNotFoundException in Jasper EL evaluation
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Mark, On 3/10/16 4:43 PM, Mark Thomas wrote: > On 10/03/2016 21:16, jimi.hulleg...@svensktnaringsliv.se wrote: >> On Thursday, March 10, 2016 11:20 AM, ma...@apache.org wrote: >>> 3. Why is the problem not limited to the first request for a jsp page? >>> >>> Because EL imports may be dynamic so the EL has to be evaluated >>> on execution. >> >> I'm not really sure I follow you now. Can you explain what you >> mean with dynamic imports in this regard? I can't see any >> mentioning of it in the specs >> (http://download.oracle.com/otn-pub/jcp/el-3_0-fr-eval-spec/EL3.0.FR. pdf). > >> > There is nothing stopping a JSP author obtaining a reference to > the ImportHandler and conditionally adding classes to import. The > configuration of the ImportHandler could change on every call to > the page. What about marking the ImportHandler as "dirty" and flushing a cache of prior lookups? (Or are we talking about spec-defined classes only, here?) - -chris -BEGIN PGP SIGNATURE- Comment: GPGTools - http://gpgtools.org Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iEYEARECAAYFAlbh8j4ACgkQ9CaO5/Lv0PDP1gCgtBnEJJA9er6w5mC1MUCLofA+ qqYAn2tBNbTYYL+fuV1rIEOklNlPOfsG =vsTe -END PGP SIGNATURE- - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Intermittent ClassNotFoundException in Jasper EL evaluation
On 10/03/2016 21:16, jimi.hulleg...@svensktnaringsliv.se wrote:
> On Thursday, March 10, 2016 11:20 AM, ma...@apache.org wrote:
>>
>>> 3. Why is the problem not limited to the first request for a jsp
>>> page?
>>
>> Because EL imports may be dynamic so the EL has to be evaluated on
>> execution.
>
> I'm not really sure I follow you now. Can you explain what you mean
> with dynamic imports in this regard? I can't see any mentioning of it
> in the specs
> (http://download.oracle.com/otn-pub/jcp/el-3_0-fr-eval-spec/EL3.0.FR.pdf).
There is nothing stopping a JSP author obtaining a reference to the
ImportHandler and conditionally adding classes to import. The
configuration of the ImportHandler could change on every call to the page.
> Can you give a concrete example, where an EL expression element
> would need to be evaluated as a potential class, again and again for
> each request?
See above.
> Because the way I see it, "foo" in ${foo.bar} in some jsp page only
> needs to be evaluated once. If "foo" corresponded to a class,
> matching one of the imports in the jsp, then this lookup will not
> change unless the jsp code changes. And the same is true if the
> lookup failes with a ClassNotFoundException. What am I missing?
See above.
>>> 4. Why isn't the ClassNotFoundException logged by the
>>> ImportHandler?
>>
>> Because it is expected and logging it provides no value to the
>> user.
>
> Well, I happen to disagree. :) In our case, if we could see all these
> stacktraces in the log, by enabling DEBUG/TRACE log level on the
> ImportHandler class for example, then we would quickly see just how
> big this problem is (ie how often this happens), on what jsp pages it
> happens, and what EL expressions cause it.
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.
> On my local machine, I was able to "catch" the ClassNotFoundException
> using a debug breakpoint in my IDE, and using the breakpoint hit
> count I could see that this exception is thrown and handled (ie
> ignored) by the ImportHandler 2700 times for a single request to the
> start page.
>
>
>> The JavaEE specs are very big on backwards compatibility.
>> Therefore, the chances of changing the spec syntax to fix this are
>> zero.
>
> OK. Fair enough. I agree with you that backwards compatibility is
> important. But there are ways to fix this while still keeping the
> backwards compatibility. For example by making it possible to turn
> off this feature (globally, per webapp, or per jsp), where the
> default is "on". Wouldn't you agree that such a change would be 100%
> backwards compatible?
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.
> And at the same time it would more or less
> solve this problem completely. Because people who experience the
> mentioned problems could turn of this setting globally, and then only
> enable it for those specific jsp pages where it is needed (and these
> pages would then be cleaned up, so that no EL-references exist
> without specific scope unless they are known to never be null. Tada,
> problem solved! =)
Um, no. See above. This would have to be a global option. It would work
for some users/pages but not for others.
> Actually... this wouldn't even need to be in the specifications... I
> can't see any harm in a EL implementation introducing such settings
> on its own, before the specifications can "catch up". That way you
> could basically introduce this fix in trunk right now, and have it
> tested and out in a stable release in no time =)
We have added such spec breaking options in the past as you'll see if
you look at the configuration docs for system properties. Generally, I'm
not a fan of configuration via system property. It is usually too blunt
a tool and doesn't provide the per webapp control that most users need.
> On Thursday, March 10, 2016 1:24 PM, ma...@apache.org wrote:
>>
>> The issue is in ScopedAttributeELResolver.
>>
>> ScopedAttributeELResolver checks the
>> page/request/session/application context first and only if it
>> doesn't find the attribute there does it try loading the class.
>
> When debugging this in my IDE, I could see this in action. I also
> noticed that the ImportHandler that performs the class lookup is
> fetched from the ELContext object. So if I could wrap that object, I
> could simply return null in the method getImportHandler(), thus
> disabling this functionallity and therefore solving the performance
> problem for us. But I couldn't find any way to wrap the ELContext,
> for some reason. Can it be done, using standard code or config?
No.
> Or can this "import logic" be disabled some other way?
You can do most things via reflection, incl
RE: Intermittent ClassNotFoundException in Jasper EL evaluation
On Thursday, March 10, 2016 11:20 AM, ma...@apache.org wrote:
>
> > 3. Why is the problem not limited to the first request for a jsp page?
>
> Because EL imports may be dynamic so the EL has to be evaluated on execution.
I'm not really sure I follow you now. Can you explain what you mean with
dynamic imports in this regard? I can't see any mentioning of it in the specs
(http://download.oracle.com/otn-pub/jcp/el-3_0-fr-eval-spec/EL3.0.FR.pdf).
Can you give a concrete example, where an EL expression element would need to
be evaluated as a potential class, again and again for each request?
Because the way I see it, "foo" in ${foo.bar} in some jsp page only needs to be
evaluated once. If "foo" corresponded to a class, matching one of the imports
in the jsp, then this lookup will not change unless the jsp code changes. And
the same is true if the lookup failes with a ClassNotFoundException. What am I
missing?
> > 4. Why isn't the ClassNotFoundException logged by the ImportHandler?
>
> Because it is expected and logging it provides no value to the user.
Well, I happen to disagree. :)
In our case, if we could see all these stacktraces in the log, by enabling
DEBUG/TRACE log level on the ImportHandler class for example, then we would
quickly see just how big this problem is (ie how often this happens), on what
jsp pages it happens, and what EL expressions cause it.
On my local machine, I was able to "catch" the ClassNotFoundException using a
debug breakpoint in my IDE, and using the breakpoint hit count I could see that
this exception is thrown and handled (ie ignored) by the ImportHandler 2700
times for a single request to the start page.
> The JavaEE specs are very big on backwards compatibility.
> Therefore, the chances of changing the spec syntax to fix this are zero.
OK. Fair enough. I agree with you that backwards compatibility is important.
But there are ways to fix this while still keeping the backwards compatibility.
For example by making it possible to turn off this feature (globally, per
webapp, or per jsp), where the default is "on". Wouldn't you agree that such a
change would be 100% backwards compatible? And at the same time it would more
or less solve this problem completely. Because people who experience the
mentioned problems could turn of this setting globally, and then only enable it
for those specific jsp pages where it is needed (and these pages would then be
cleaned up, so that no EL-references exist without specific scope unless they
are known to never be null. Tada, problem solved! =)
Actually... this wouldn't even need to be in the specifications... I can't see
any harm in a EL implementation introducing such settings on its own, before
the specifications can "catch up". That way you could basically introduce this
fix in trunk right now, and have it tested and out in a stable release in no
time =)
On Thursday, March 10, 2016 1:24 PM, ma...@apache.org wrote:
>
> The issue is in ScopedAttributeELResolver.
>
> ScopedAttributeELResolver checks the page/request/session/application context
> first and only if it doesn't find the attribute there does it try loading the
> class.
When debugging this in my IDE, I could see this in action. I also noticed that
the ImportHandler that performs the class lookup is fetched from the ELContext
object. So if I could wrap that object, I could simply return null in the
method getImportHandler(), thus disabling this functionallity and therefore
solving the performance problem for us. But I couldn't find any way to wrap the
ELContext, for some reason. Can it be done, using standard code or config? Or
can this "import logic" be disabled some other way?
There really should be a way for users to disable this, if the functionallity
is not used and it just is causing problems. And, like I said above, that could
be done without breaking the specs. And an alternative way to the way above,
could be like I mentioned before, to add a configuration option that forces the
class name to begin with a capital letter. That way ${Boolean.TRUE},
${Integer.MAX_VALUE} and ${MyClass.myStaticField} would still work, while
${foo.bar} etc would simply be ignored. As long as the configuration option
would default to false (ie lower case first letter is allowed, as per the
specification) it wouldn't break the specification unless the user deliberately
told it to (which is fine, right?).
It would be really nice to get your input on these suggestions. And if you
don't like them, could you explain why? If your opinion is "We need to stick
100% to the specification, and never ever give even the expert user any way to
override this, ever", then I would say that such a view causes more harm than
good. :)
Regards
/Jimi
-
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Intermittent ClassNotFoundException in Jasper EL evaluation
On 10/03/2016 10:19, Mark Thomas wrote:
> On 10/03/2016 08:12, jimi.hulleg...@svensktnaringsliv.se wrote:
>> Then surely one can look at the other implementations, and what they did to
>> avoid this problem. But one thing off the top of my head would be to at
>> least avoid doing that class lookup in cases where it couldn't be a static
>> field reference (like ${name}, since there is no dot after "name", there is
>> no reason to check if "name" is a class reference, right?).
>
> It has been a while since I looked at this. I need to remind myself why
> I thought it needed to be implemented this way. I recall looking at this
> at least once before but another look wouldn't hurt.
Having reviewed this, here is some useful background.
The issue is in ScopedAttributeELResolver.
ScopedAttributeELResolver checks the page/request/session/application
context first and only if it doesn't find the attribute there does it
try loading the class.
If ScopedAttributeELResolver is asked to resolve "foo" it can't
differentiate between ${ foo } which doesn't require a class lookup and
${ foo.bar } which does require a class lookup. Therefore
ScopedAttributeELResolver will always do a class lookup if it didn't
find an a matching attribute.
These lookups pass through various standard APIs (EL and JSP specs) so
we can't simply add a flag to the method to indicate if a class lookup
is required or not.
We can't use ThreadLocals because either:
- the ThreadLocal would need to be defined in a specification class
which isn't allowed because we can't change the public API; or
- the ThreadLocal would need to be defined in Tomcat and referenced in
a spec class which isn't allowed because the spec JARs have to be
independent.
However, all is not lost. I think I have a solution based on ELContext
getContext()/putContext(). Keep an eye on
https://bz.apache.org/bugzilla/show_bug.cgi?id=57583 for progress reports.
-
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Intermittent ClassNotFoundException in Jasper EL evaluation
On 10/03/2016 08:12, jimi.hulleg...@svensktnaringsliv.se wrote:
> On Wednesday, March 9, 2016 8:22 PM, ma...@apache.org wrote:
>> It is a known 'feature' of the new EL requirements added in 3.0. The EL
>> parser can't differentiate
>> between an attribute without a scope and a reference to an static field.
>>
>> See https://bz.apache.org/bugzilla/show_bug.cgi?id=57583
>
> Interesting. I can see how that in a way could explain the the performance
> regression. But what I don't understand is:
>
> 1. Why would this cause a ClassNotFoundException? What class exactly is it
> trying to load?
For each imported package, it is trying to load package.attributeName
> 2. Why is this happening seemingly intermittently, with different EL
> variables each time?
It isn't. It is happening every time. You are just observing it
intermittently. Probably because class loading is serial by default and
how observable this is depends on how many threads are trying to load
classes in parallel. Note that 9.0.x will use ParallelWebappClassLoader
by default from the next release and you can always switch to that class
loader in 8.0.x
> 3. Why is the problem not limited to the first request for a jsp page? We see
> this problem even days after a restart, for jsp-pages that definitely have
> been used multiple times already, with the same state for it's variables.
Because EL imports may be dynamic so the EL has to be evaluated on
execution.
> 4. Why isn't the ClassNotFoundException logged by the ImportHandler?
Because it is expected and logging it provides no value to the user.
> 5. Why would this change between Tomcat 7 and Tomcat 8, with the exact same
> webapp war with the exact same web.xml?
Because imports and statics were added in EL 3.0 which was first
implemented in Tomcat 8.
> 6. In our web.xml we specify version="2.5". Wouldn't that mean EL version
> 2.1? (http://tomcat.apache.org/whichversion.html)
No. The version you specify in web.xml only determines the rules that
Tomcat uses to validate the web.xml. It has zero impact on the behaviour
of the container. The overall JavaEE EG made it very clear that this is
how they expected implementations to behave.
>> The way to avoid it is to always add an explicit scope (page, request,
>> application, session) to your attributes.
>
> Is this an official recommendation, stemming from the EL 3.0 specifications?
> If so, can you point me to that paragraph in the specification document, or
> some other paper of similar nature? Because when I look at the 3.0
> specification document, all I see is examples without scope.
>
> Or is this just a pragmatic response to the Tomcat/Jasper implementation of
> the specifications?
It is the best recommendation I have right now based on what I know
about the EL spec and Tomcat's implementation of it.
> The reason I ask is that a simple search in our code base show that we have
> about 10.000 potential candidates for the change you are suggesting, spread
> out in hundreds of jsp files in our project. And on top of that, the CMS that
> we use have their own jsp files, and a quick check indicates thousands of
> potential candidates for the change there as well. So not only would we have
> to perform a monumental task in our own code (because we would need to
> determine the scope manually, for each and every variable), we also would
> need to ask the CMS company to perform the same task.
You aren't the only one in that position.
>> Suggestions for improvements to the default ImportHandler implementation to
>> make this faster are welcome.
>
> Well, I am quite pragmatic in my thinking. Is this EL implementation the only
> implementation with this problem?
I don't know. There aren't that many JSP implementations out there. The
end result should be the same with all of them but how they get there
may vary.
> Then surely one can look at the other implementations, and what they did to
> avoid this problem. But one thing off the top of my head would be to at least
> avoid doing that class lookup in cases where it couldn't be a static field
> reference (like ${name}, since there is no dot after "name", there is no
> reason to check if "name" is a class reference, right?).
It has been a while since I looked at this. I need to remind myself why
I thought it needed to be implemented this way. I recall looking at this
at least once before but another look wouldn't hurt.
> Otherwise, if more or less all implementations suffer from this problem, then
> maybe it is the specification that is to blame. Maybe, when introducing the
> new concept of EL references to static fields, they could use a special
> prefix, like ${static.Boolean.True}, or they could have had this feature
> turned off by default, with the possibility to turn it on either per jsp page
> (using some page directive like <%@ page staticElReferences="true" %>) or
> webapp-globally in the web.xml. Or, they could simply include the requirement
> t
RE: Intermittent ClassNotFoundException in Jasper EL evaluation
On Wednesday, March 9, 2016 8:22 PM, ma...@apache.org wrote:
> It is a known 'feature' of the new EL requirements added in 3.0. The EL
> parser can't differentiate
> between an attribute without a scope and a reference to an static field.
>
> See https://bz.apache.org/bugzilla/show_bug.cgi?id=57583
Interesting. I can see how that in a way could explain the the performance
regression. But what I don't understand is:
1. Why would this cause a ClassNotFoundException? What class exactly is it
trying to load?
2. Why is this happening seemingly intermittently, with different EL variables
each time?
3. Why is the problem not limited to the first request for a jsp page? We see
this problem even days after a restart, for jsp-pages that definitely have been
used multiple times already, with the same state for it's variables.
4. Why isn't the ClassNotFoundException logged by the ImportHandler?
5. Why would this change between Tomcat 7 and Tomcat 8, with the exact same
webapp war with the exact same web.xml?
6. In our web.xml we specify version="2.5". Wouldn't that mean EL version 2.1?
(http://tomcat.apache.org/whichversion.html)
> The way to avoid it is to always add an explicit scope (page, request,
> application, session) to your attributes.
Is this an official recommendation, stemming from the EL 3.0 specifications? If
so, can you point me to that paragraph in the specification document, or some
other paper of similar nature? Because when I look at the 3.0 specification
document, all I see is examples without scope.
Or is this just a pragmatic response to the Tomcat/Jasper implementation of the
specifications?
The reason I ask is that a simple search in our code base show that we have
about 10.000 potential candidates for the change you are suggesting, spread out
in hundreds of jsp files in our project. And on top of that, the CMS that we
use have their own jsp files, and a quick check indicates thousands of
potential candidates for the change there as well. So not only would we have to
perform a monumental task in our own code (because we would need to determine
the scope manually, for each and every variable), we also would need to ask the
CMS company to perform the same task.
> Suggestions for improvements to the default ImportHandler implementation to
> make this faster are welcome.
Well, I am quite pragmatic in my thinking. Is this EL implementation the only
implementation with this problem? Then surely one can look at the other
implementations, and what they did to avoid this problem. But one thing off the
top of my head would be to atleast avoid doing that class lookup in cases where
it couldn't be a static field reference (like ${name}, since there is no dot
after "name", there is no reason to check if "name" is a class reference,
right?).
Otherwise, if more or less all implementations suffer from this problem, then
maybe it is the specification that is to blame. Maybe, when introducing the new
concept of EL references to static fields, they could use a special prefix,
like ${static.Boolean.True}, or they could have had this feature turned off by
default, with the possibility to turn it on either per jsp page (using some
page directive like <%@ page staticElReferences="true" %>) or webapp-globally
in the web.xml. Or, they could simply include the requirement that the class
name must start with a capital letter, thus only causing problems for people
who break the coding standard (ie either having a class name starting with a
lower case letter, or having a variable name starting with an upper case
letter).
/Jimi
-
To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org
For additional commands, e-mail: users-h...@tomcat.apache.org
Re: Intermittent ClassNotFoundException in Jasper EL evaluation
On 09/03/2016 18:20, jimi.hulleg...@svensktnaringsliv.se wrote: > Has anyone seen this problem before? What could be the cause of it? It is a known 'feature' of the new EL requirements added in 3.0. The EL parser can't differentiate between an attribute without a scope and a reference to an static field. See https://bz.apache.org/bugzilla/show_bug.cgi?id=57583 The way to avoid it is to always add an explicit scope (page, request, application, session) to your attributes. Suggestions for improvements to the default ImportHandler implementation to make this faster are welcome. Mark - To unsubscribe, e-mail: users-unsubscr...@tomcat.apache.org For additional commands, e-mail: users-h...@tomcat.apache.org
