Hi Kevin,
On 03.04.2020 01:21, Kevin Rushforth wrote:
I see that you updated the PR and sent it for review.
Before we formally review it in the PR, let's finish the discussion as to
whether this is a useful
feature, and if so, what form this feature should take.
From my point of view, this does seem like a useful feature. Would other users
of FXML benefit
from it?
Script code should be executed faster after compilation, so any FXML page that
hosts script code may
benefit.
The benefits depend on the type of script (and maybe its size and its
complexity) and also on the
types of event handlers the scripts serve, e.g. move or drag event handlers may
benefit
significantly. This is because repeated invocation of compiled script event
handlers do not cause
the reparsing of that script's source and interpreting it on each invocation,
which may be expensive
depending on the script engine, but rather allows the immediate
evaluation/execution of the compiled
script by the script engine.
I'm not certain whether we want it to be implicit, compiling the script if the
script engine in
question implements Compilable, or via a new keyword or tag. What are the pros
/ cons?
In principle there are three possibilities:
1) If a script engine implements javax.script.Compilable, compile the
script and execute the
compiled version. In the case of event handlers compile and buffer the
compiled script and
execute the compiled script each time its registered event fires.
o Pro: immediately benefits all existing FXML pages that host scripts
o Con: it is theoretically possible (albeit quite unlikely) that there
are scripts that fail
compiling but have been employed successfully in interpreted mode
2) Introduce some form of an optional attribute (e.g.
"compile={true|false}") to the FXML
language PI that switches on compilation of scripts hosted in FXML
definitions if the script
engine implements the javax.script.Compilable interface. If missing it would default
to "false".
(Alternatively, add a "compile" PI, that if present causes the compilation
of scripts, if the
script engine supports it. It would be an error if the "compile" PI was
present, but the
"language" PI was not.)
o Pro: compilation of FXML hosted scripts is done only, if the FXML
definition of the language
PI gets changed
o Con: benefit not made available automatically to existing FXML pages
that host scripts
3) Another possibility would be to define a boolean attribute/property
"compile" for script and
node elements and only compile the scripts, if the property is set to true
o Pro: compilation of FXML hosted scripts is done only, if the FXML
definition gets changed
accordingly
o Con: potential benefit not made available automatically to existing
FXML pages that host scripts
2 and 3 could be combined, where 2 would define the default compilation
behavior that then could be
overruled individually by 3.
The question would be whether 2 or/and 3 are really necessary as it can be
expected that compilation
of scripts by the script engines would find the same errors as while
interpreting the very same
scripts (if not, the script engine is badly broken and it could be argued that
then the
interpretation part of the script engine might be expected to be broken as well
which would be quite
dangerous from an integrity POV; the same consideration applies as well if the
execution of the
compiled script would behave differently to interpreting the very same script
by the same script
engine).
The current WIP implements 1 above and includes an appropriate test unit.
What do others think?
In either case, we would need to make sure that this doesn't present any
security concerns in the
presence of a security manager. As long as a user-provided class is on the
stack, it will be fine,
but we would need to ensure that.
The compilation of scripts needs to be done by the Java script engines
(implementing both,
javax.script.Engine and javax.script.Compilable) as well as controlling the
execution of compiled
scripts ([javax.script.CompiledScript]
(https://docs.oracle.com/en/java/javase/14/docs/api/java.scripting/javax/script/CompiledScript.html)).
---rony
On 4/2/2020 10:41 AM, Rony G. Flatscher wrote:
After merging master, applying some fixes and changing the title to reflect the
change from WIP to a
pull request I would kindly request a review of this pull request!
Here the URL: <https://github.com/openjdk/jfx/pull/129>, title changed to:
"8238080: FXMLLoader: if
script engines implement javax.script.Compilable compile scripts".
---rony
On 28.02.2020 19:22, Rony G. Flatscher wrote:
Here is a WIP [1] implementation of [2]. The WIP is based on [3], which is
currently in review, and
has an appropriate test unit going with it as well, please review.
There should be no compatibility issue with this implementation.
Discussion: another solution could be to not compile by default. Rather
compile, if some new
information is present with the FXML file which cannot possibly be present in
existing FXML files.
In this scenario one possible and probably simple solution would be to only
compile scripts if the
language process instruction (e.g. <?language rexx?>) contains an appropriate
attribute with a
value
indicating that compilation should be carried out (e.g.: compile="true"). This
way only FXML with
PIs having this attribute set to true would be affected. If desired I could try
to come up with a
respective solution as well.
---rony
[1] "Implementation and test unit": <https://github.com/openjdk/jfx/pull/129>
[2] "JDK-8238080 : FXMLLoader: if script engines implement
javax.script.Compilable compile
scripts":
<https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8238080>
[3] "8234959: FXMLLoader does not populate ENGINE_SCOPE Bindings with FILENAME and
ARGV":
<https://github.com/openjdk/jfx/pull/122/commits>
On 24.01.2020 16:26, Kevin Rushforth wrote:
Thank you for filing this enhancement request. As an enhancement it should be
discussed on this
list before proceeding with a pull request (although a "WIP" or Draft PR can be
used to illustrate
the concept).
For my part, this seems like a reasonable enhancement, as long as there are no
compatibility
issues, but it would be good to hear from application developers who heavily
use FXML.
-- Kevin
On 1/24/2020 7:21 AM, Rony G. Flatscher wrote:
Just filed a RFE with the following information:
* Component:
o JavaFX
* Subcomponent:
o fxml: JavaFX FXML
* Synopsis:
o "FXMLLoader: if script engines implement javax.script.Compilabel compile
scripts"
* Descriptions:
o "FXMLLoader is able to execute scripts in Java script languages
(javax.script.ScriptEngine
implementations) if such a Java script language gets defined as the
controller
language in
the FXML file.
If a script engine implements the javax.script.Compilable interface,
then such scripts
could
be compiled and the resulting javax.script.CompiledScript could be
executed instead
using
its eval() methods.
Evaluating the CompiledScript objects may help speed up the
execution of script
invocations,
especially for scripts defined for event attributes in FXML elements
(e.g. like
onMouseMove)
which may be repetitevly invoked and evaluated."
* System /OS/Java Runtime Information:
o "All systems."
Received the internal review ID: "9063426"
---rony
