Hi there,
as there was probably enough time that has passed by I would intend to create a
CSR in the next days
with the PR as per Kevin's suggestion.
(For the case that this feature should not be active by default, the CSR will
suggest to define a
new "compile" PI in the form <?compile [true|false] ?> (default, if no PI data
given: true), which
is independent of the existence of a language PI (this way it becomes also
possible to allow
compilation of external scripts denoted with the script-element, which do not
need a page language
to be set as the file's extension allows the appropriate script engine to be
loaded and used for
execution). A compile-PI would allow for turning compilation of scripts on by
just adding the PI
<?compile?> or <?compile true?> to FXML files (and <?compile false?> to turn
off), which seems to
be simple and self-documentary. In general employing such compile PIs allows
for setting compilation
of scripts on and off throughout an FXML file.)
---rony
On 04.04.2020 18:03, Rony G. Flatscher wrote:
> 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
>
