https://bz.apache.org/bugzilla/show_bug.cgi?id=68596

--- Comment #6 from Konstantin Kolinko <knst.koli...@gmail.com> ---
(In reply to John Engebretson from comment #0)
>     context.setPropertyResolved(false);
>     ...
> 
> context can be of type ELContextImpl, ELContextWrapper, EvaluationContext,
> or StandardELContext so this method triggers an invokevirtual.

The setPropertyResolved(boolean) method is not overwritten in subclasses. This
is a common situation that occurs everywhere, thus I wonder why this specific
JVM cannot optimize it. 

The only unusual is that the method is overloaded, having
"setPropertyResolved(boolean)" and "setPropertyResolved(Object base, Object
property)".


(In reply to Mark Thomas from comment #2)
> #1 isn't an option unfortunately. With more complex EL expressions
> ELContext.isPropertyResolved() will return true at the start of the call to
> convertToType(). At least one test fails if this code is removed.

Regarding #1. Looking at source code in current main branch,

1) Comparing ELContext.convertToType(..) and ELResolver.convertToType(..) I
wonder whether the only proper public entry point is the ELContext method.

In other words, I mean that ELResolver.convertToType(...) methods might be an
implementation detail, and should not be called as a public API.

A weak point in my logic is that ELResolver.convertToType(...) methods are
declared "public" and not "protected". (Also, I have not looked into the
specification PDF.)


The ELContext.convertToType() method has logic that clears and restores its
"resolved" flag. Thus ELResolver.convertToType(...) implementations could
assume that the flag has been cleared and skip calling
"setPropertyResolved(false)".

As in comment #2 Mark said "At least one test fails if this code is removed."
either my logic is wrong, or there is a bug, or maybe an incorrect test.


2) If my "1)" is wrong then I think that there is a bug in OptionalELResolver:

Its convertToType(...) method is missing the
"context.setPropertyResolved(false);" call.


3) If we talk about minor optimizations:

I do not believe in the following proposals, but maybe you'll want to test
them:

- Calling setPropertyResolved(false); may be skipped if the flag is already
false.

It is trivial to implement this optimization in ELContext.convertToType(), as
it has already read the value as "originalResolved".

Implementing the same in CompositeELResolver means adding a call to
isPropertyResolved(), but I think it might be cheaper.

- I think that calling isPropertyResolved() may be skipped if result is
non-null.

I.e. "if (context.isPropertyResolved()) { return result; }"
could be rewritten as
"if (result != null || context.isPropertyResolved()) { return result; }"

A downside is that getting a non-null result is rare, thus this optimization
will be useless.

-- 
You are receiving this mail because:
You are the assignee for the bug.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to