Hi,
I am currently working on a number of smaller performance improvements, and
I found that some BindingsValuesProvider could benefit from providings
lazily. In that case the binding is added, but the value is only resolved
when the value of the binding is actually needed, instead of resolving them
when adding the binding [1]. As these bindings are added very often, but
not always used, a small improvement in the area of a few hundred
microseconds can accumulate to a 2-digit number of milliseconds for a
complex request;
The implementations of the BindingValuesProvider#addBindings() currently
often perform checks if the binding should be added at all. In the case of
a failed check the binding would not be added at all.
With the dynamic resolution this is hardly possible, it would defeat the
purpose of delaying the resolution (including these checks) to a time when
it's absolutely necessary. In the case of the dynamic resolution with a
failed check this just leaves the option to return "null" for that binding.
The javax.script.Bindings#get spec [2] allows to return null in 2 cases:
* the mapping is not present
* the mapping returns a null
So technically this change is covered by the spec, but I don't know if all
users of the bindings always check for null but have used some other
approach instead:
if (bindings.containsKey(BINDING)) {
Object o = bindings.get(BINDING);
assertNotNull(o);
}
which is incorrect according to the spec, but might have worked in the past.
In my opinion we should adjust these BindingsValuesProviders and require
the consumers of these bindings to make sure that they do the null-check as
required by the spec.
WDYT?
Jörg
[1]
https://github.com/apache/sling-org-apache-sling-api/blob/master/src/main/java/org/apache/sling/api/scripting/LazyBindings.java
[2]
https://docs.oracle.com/javase/8/docs/api/javax/script/Bindings.html#get-java.lang.Object-
--
Cheers,
Jörg Hoh,
https://cqdump.joerghoh.de
Twitter: @joerghoh
