You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Rainer Jung <ra...@kippdata.de> on 2016/02/03 16:48:48 UTC
ELParser fiel of type LookaheadSuccess extends Error
Hi there,
ELParser has a field named jj_ls of type LookaheadSuccess which extends
Error. It is created during each instantiation of an ELParser object.
Creating an Error is quite slow, because e.g. it calls
java.lang.Throwable.fillInStack() during init.
This jj_ls is only used once in jj_scan_token() where it is thrown in
case of certain condition, see line 2985:
2968 private boolean jj_scan_token(int kind) {
2969 if (jj_scanpos == jj_lastpos) {
2970 jj_la--;
2971 if (jj_scanpos.next == null) {
2972 jj_lastpos = jj_scanpos = jj_scanpos.next =
token_source.getNextToken();
2973 } else {
2974 jj_lastpos = jj_scanpos = jj_scanpos.next;
2975 }
2976 } else {
2977 jj_scanpos = jj_scanpos.next;
2978 }
2979 if (jj_rescan) {
2980 int i = 0; Token tok = token;
2981 while (tok != null && tok != jj_scanpos) { i++; tok = tok.next; }
2982 if (tok != null) jj_add_error_token(kind, i);
2983 }
2984 if (jj_scanpos.kind != kind) return true;
2985 if (jj_la == 0 && jj_scanpos == jj_lastpos) throw jj_ls;
2986 return false;
2987 }
This is generated code and it is the only place were jj_ls is being used.
I noticed this situation because I'm doing a performance analysis of an
application. I wonder whether creating this jj_ls for each new ELParser
instance in advance is the right thing to do. One could also put the
burden on the usage side, e.g. generating it when it is needed to be
thrown. Unfortunately I don't know enough about the typical runtime
details of ELParser. E.g. it might be that typically only few instances
of it are created because they get reused (which doesn't seem to be the
case, we only create and use it method local in
createNodeInternal.createNodeInternal()) or their results get cached
(which seems to be the case).
I'll have a look at how effective the caching is for the application in
question, but I'd be interested if there is an opinion whether the
pattern to create the Error object eagerly when creating the ELParser
object seems appropriate. No idea currently, how one would change it
though, because it is generated code.
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: ELParser fiel of type LookaheadSuccess extends Error
Posted by Rainer Jung <ra...@kippdata.de>.
Hi Chris,
Am 03.02.2016 um 17:04 schrieb Christopher Schultz:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Rainer,
>
> On 2/3/16 10:48 AM, Rainer Jung wrote:
>> Hi there,
>>
>> ELParser has a field named jj_ls of type LookaheadSuccess which
>> extends Error. It is created during each instantiation of an
>> ELParser object. Creating an Error is quite slow, because e.g. it
>> calls java.lang.Throwable.fillInStack() during init.
>>
>> This jj_ls is only used once in jj_scan_token() where it is thrown
>> in case of certain condition, see line 2985:
>>
>> 2968 private boolean jj_scan_token(int kind) { 2969 if (jj_scanpos
>> == jj_lastpos) { 2970 jj_la--; 2971 if (jj_scanpos.next == null) {
>> 2972 jj_lastpos = jj_scanpos = jj_scanpos.next =
>> token_source.getNextToken(); 2973 } else { 2974 jj_lastpos =
>> jj_scanpos = jj_scanpos.next; 2975 } 2976 } else { 2977 jj_scanpos
>> = jj_scanpos.next; 2978 } 2979 if (jj_rescan) { 2980 int i = 0;
>> Token tok = token; 2981 while (tok != null && tok != jj_scanpos) {
>> i++; tok = tok.next; } 2982 if (tok != null)
>> jj_add_error_token(kind, i); 2983 } 2984 if (jj_scanpos.kind !=
>> kind) return true; 2985 if (jj_la == 0 && jj_scanpos == jj_lastpos)
>> throw jj_ls; 2986 return false; 2987 }
>>
>> This is generated code and it is the only place were jj_ls is being
>> used.
>>
>> I noticed this situation because I'm doing a performance analysis
>> of an application. I wonder whether creating this jj_ls for each
>> new ELParser instance in advance is the right thing to do. One
>> could also put the burden on the usage side, e.g. generating it
>> when it is needed to be thrown. Unfortunately I don't know enough
>> about the typical runtime details of ELParser. E.g. it might be
>> that typically only few instances of it are created because they
>> get reused (which doesn't seem to be the case, we only create and
>> use it method local in createNodeInternal.createNodeInternal()) or
>> their results get cached (which seems to be the case).
>>
>> I'll have a look at how effective the caching is for the
>> application in question, but I'd be interested if there is an
>> opinion whether the pattern to create the Error object eagerly when
>> creating the ELParser object seems appropriate. No idea currently,
>> how one would change it
>
> Is the stack trace of the current implementation important when that
> Error (subclass) is thrown? If the stack trace is important to be
> filled-in at that earlier time, then this is a complex issue. If
> nobody cares what the stack trace says, I'm +1 for only instantiating
> the Error when it needs to be thrown.
The stack should be nearly the same except for some <init> on top
currently and jj_scan_token() when instantiated on demand. Is it
important? I'd say the frames lower down should help people to find
where in the app they create an EL which on parsing throws the error.
I'd be interested if there's an expectation whether ELParser object
creation could happen frequently or should happen rarely (after an app
has reached steady state).
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: ELParser fiel of type LookaheadSuccess extends Error
Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Rainer,
On 2/3/16 10:48 AM, Rainer Jung wrote:
> Hi there,
>
> ELParser has a field named jj_ls of type LookaheadSuccess which
> extends Error. It is created during each instantiation of an
> ELParser object. Creating an Error is quite slow, because e.g. it
> calls java.lang.Throwable.fillInStack() during init.
>
> This jj_ls is only used once in jj_scan_token() where it is thrown
> in case of certain condition, see line 2985:
>
> 2968 private boolean jj_scan_token(int kind) { 2969 if (jj_scanpos
> == jj_lastpos) { 2970 jj_la--; 2971 if (jj_scanpos.next == null) {
> 2972 jj_lastpos = jj_scanpos = jj_scanpos.next =
> token_source.getNextToken(); 2973 } else { 2974 jj_lastpos =
> jj_scanpos = jj_scanpos.next; 2975 } 2976 } else { 2977 jj_scanpos
> = jj_scanpos.next; 2978 } 2979 if (jj_rescan) { 2980 int i = 0;
> Token tok = token; 2981 while (tok != null && tok != jj_scanpos) {
> i++; tok = tok.next; } 2982 if (tok != null)
> jj_add_error_token(kind, i); 2983 } 2984 if (jj_scanpos.kind !=
> kind) return true; 2985 if (jj_la == 0 && jj_scanpos == jj_lastpos)
> throw jj_ls; 2986 return false; 2987 }
>
> This is generated code and it is the only place were jj_ls is being
> used.
>
> I noticed this situation because I'm doing a performance analysis
> of an application. I wonder whether creating this jj_ls for each
> new ELParser instance in advance is the right thing to do. One
> could also put the burden on the usage side, e.g. generating it
> when it is needed to be thrown. Unfortunately I don't know enough
> about the typical runtime details of ELParser. E.g. it might be
> that typically only few instances of it are created because they
> get reused (which doesn't seem to be the case, we only create and
> use it method local in createNodeInternal.createNodeInternal()) or
> their results get cached (which seems to be the case).
>
> I'll have a look at how effective the caching is for the
> application in question, but I'd be interested if there is an
> opinion whether the pattern to create the Error object eagerly when
> creating the ELParser object seems appropriate. No idea currently,
> how one would change it
Is the stack trace of the current implementation important when that
Error (subclass) is thrown? If the stack trace is important to be
filled-in at that earlier time, then this is a complex issue. If
nobody cares what the stack trace says, I'm +1 for only instantiating
the Error when it needs to be thrown.
- -chris
-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - http://gpgtools.org
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/
iEYEARECAAYFAlayJRcACgkQ9CaO5/Lv0PDawQCfbvmI12paCRrzqh4sArOg+ImQ
MHIAoIMAiGTtFXzL/GEYhHLbLvVF1uVY
=tfnY
-----END PGP SIGNATURE-----
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: ELParser fiel of type LookaheadSuccess extends Error
Posted by Rainer Jung <ra...@kippdata.de>.
Am 03.02.2016 um 16:48 schrieb Rainer Jung:
> Hi there,
>
> ELParser has a field named jj_ls of type LookaheadSuccess which extends
> Error. It is created during each instantiation of an ELParser object.
> Creating an Error is quite slow, because e.g. it calls
> java.lang.Throwable.fillInStack() during init.
...
> I noticed this situation because I'm doing a performance analysis of an
> application. I wonder whether creating this jj_ls for each new ELParser
> instance in advance is the right thing to do. One could also put the
> burden on the usage side, e.g. generating it when it is needed to be
> thrown. Unfortunately I don't know enough about the typical runtime
> details of ELParser. E.g. it might be that typically only few instances
> of it are created because they get reused (which doesn't seem to be the
> case, we only create and use it method local in
> createNodeInternal.createNodeInternal()) or their results get cached
> (which seems to be the case).
>
> I'll have a look at how effective the caching is for the application in
> question, but I'd be interested if there is an opinion whether the
> pattern to create the Error object eagerly when creating the ELParser
> object seems appropriate. No idea currently, how one would change it
> though, because it is generated code.
Update: The case in question was triggered by an app having a big number
of EL expressions, about 25.000. So this could be handled by increasing
the org.apache.el.ExpressionBuilder.CACHE_SIZE.
To me it looks like the current ConcurrentCache code, which moves every
item from the first level cache "eden" to the second level cache
"longterm" when "eden" is full, is simple, but probably often not very
effective. Since longterm is a WeakHashMap I would expect the entries in
"logterm" to get destroyed quickly after the page is done. So once your
cache size gets to small, I expect more or less the cache to behave as
being completely purged. Then it starts to fill up again (and get more
effective) until the same pattern hits. So I don't expect a big use of
"longterm" depending on your GC rate and eden size. If you are lucky,
some stuff from "longterm" gets promoted to tenured and might live
longer there, but it could well be, that "longterm" dies quickly in the
young generation.
I don't have a better and simple alternative pattern at hand. It could
be interesting to make the current "eden" cache size retrievable via
JMX, but unfortunately it seems size() can be an expensive operation for
a ConcurrentHashMap under contended conditions. Not sure, whether we
should expose it, because then it will be called fir each monitoring cycle.
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: ELParser fiel of type LookaheadSuccess extends Error
Posted by Konstantin Kolinko <kn...@gmail.com>.
2016-02-03 18:48 GMT+03:00 Rainer Jung <ra...@kippdata.de>:
> Hi there,
>
> ELParser has a field named jj_ls of type LookaheadSuccess which extends
> Error. It is created during each instantiation of an ELParser object.
> Creating an Error is quite slow, because e.g. it calls
> java.lang.Throwable.fillInStack() during init.
Project home for JavaCC + JJTree:
https://javacc.java.net/
https://java.net/projects/javacc
Looking into source code of their trunk,
\src\main\java\org\javacc\parser\ParseGen.java
[[[
if (jj2index != 0) {
genCodeLine(" @SuppressWarnings(\"serial\")");
genCodeLine(" static private final class
LookaheadSuccess extends "+(Options.isLegacyExceptionHandling() ?
"java.lang.Error" : "java.lang.RuntimeException")+" { }");
genCodeLine(" " + staticOpt()
+ "final private LookaheadSuccess jj_ls = new
LookaheadSuccess();");
]]]
and \src\main\java\org\javacc\parser\JavaCCGlobals.java
[[[
static public String staticOpt() {
if (Options.getStatic()) {
return "static ";
} else {
return "";
}
}
]]]
So
1. There is an option that adds "static" modifier to this field, and
actually to all parser methods.
It should be good to try that.
There is also Options.isLegacyExceptionHandling() in the above code
fragment, though it does not matter for this issue.
Generally, it will be good to document how these sources are generated.
2. It is known that when using a cached exception instance like that,
one must overwrite the fillInStackTrace() method.
https://bz.apache.org/bugzilla/show_bug.cgi?id=50460
https://issues.apache.org/jira/browse/XERCESJ-1667
The javacc does not have such feature. (We may propose a patch to
them, but who knows when it will be applied).
Assuming that the code is regenerated with "static" option on, we can
a) patch the generated code by implementing fillInStackTrace() method,
b) preload the class.
If we patch, we can also do so without regenerating the code.
A well known place that preloads classes is
org.apache.jasper.security.SecurityClassLoad class, but it preloads
classes only when SecurityManager is present.
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org