Next steps ? (Re: An attempt of a CSR draft ... (Re: A new WIP (PR # 192) (Re: WIP version with PI compile (Re: A WIP for JDK-8238080 for review/discussion (Re: "Internal review ID 9063426: "FXMLLoader: if script engines implement javax.script.Compilabel compile scripts"
Rony G Flatscher
rony.flatscher at wu.ac.at
Sat May 23 18:30:05 UTC 2020
Hi Kevin,
is there anything I need to do? What are the next steps I should look for?
—-rony
Rony G. Flatscher (mobil/e)
> Am 12.05.2020 um 19:23 schrieb Rony G. Flatscher <Rony.Flatscher at wu.ac.at>:
>
> Hi Kevin,
>
> in the meantime I have tried to come up with a formulation for the "Introduction to FXML"
> specification about the new compile processing instruction which is brief and complete. While being
> there I fixed a typo in the document and added a missing language processing instruction to an
> existing script example, hope that was o.k. as I have not filed an explicit bug for it.
>
> Also closed the PR 129 and 187, removed the WIP from PR 192, and supplied the CSR draft as a comment.
>
> If there is anything else I should do, please let me know.
>
> Cheers,
>
> ---rony
>
>
>> On 11.05.2020 14:12, Rony G. Flatscher wrote:
>> Hi Kevin,
>>
>>> On 09.05.2020 17:16, Kevin Rushforth wrote:
>>> I'm finally getting back to this. I took a look at https://github.com/openjdk/jfx/pull/192 and I
>>> like that as the direction for this enhancement.
>>>
>>> The initial CSR you have is a good start.
>> Thank you!
>>
>>> Next steps are:
>>>
>>> 1. Update the "Introduction to FXML" specification (see my comment in the PR)
>>> 2. Update PR 192 with the draft CSR as a comment, modifying it to include the above additions to
>>> "Introduction to FXML"
>>> 3. Remove WIP from the title
>>>
>>> You can then close the other two PRs (129 and 187).
>> will do (may take a little bit).
>>
>> Cheers
>>
>> ---rony
>>
>>
>>> On 4/28/2020 6:15 AM, Rony G. Flatscher wrote:
>>>> Hi Kevin,
>>>>
>>>> what should be the next steps?
>>>>
>>>> Should I remove "WIP" from the title in <https://github.com/openjdk/jfx/pull/192> and add the CSR
>>>> draft text of my last e-mail as a "CSR" comment with PR # 192, thereby requesting it to be added
>>>> to <https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8238080>?
>>>>
>>>> Please advise.
>>>>
>>>> TIA,
>>>>
>>>> ---rony
>>>>
>>>> P.S.: This is the RFE:
>>>>
>>>> * RFE (2020-01-24): <https://bugs.java.com/bugdatabase/view_bug.do?bug_id=JDK-8238080>
>>>>
>>>> These are the three versions (all with appropriate unit tests) that I came up with
>>>> chronologically to implement the RFE, would prefer the latest version (PR # 192):
>>>>
>>>> * Compile if Compilable implemented (2020-02-28): <https://github.com/openjdk/jfx/pull/129>
>>>> * Compile if compile PI and Compilable is implemented (2020-04-11):
>>>> <https://github.com/openjdk/jfx/pull/187>
>>>> * Compile with fallback, if Compilable is implemented, compile PI for fine-grained control
>>>> (2020-04-14): <https://github.com/openjdk/jfx/pull/192>
>>>>
>>>>
>>>> On 22.04.2020 20:01, Rony G. Flatscher wrote:
>>>>> Hi Kevin,
>>>>>
>>>>> as I am not able to file a CSR with the issue you suggested to come up with a draft, so here it goes:
>>>>>
>>>>> Summary
>>>>> =======
>>>>> Have javafx.fxml.FXMLLoader compile FXML scripts before evaluating them, if the script engine
>>>>> implements the javax.script.Compilable interface to speed up execution. In case compilation
>>>>> throws a javax.script.ScriptException fall back to evaluating the uncompiled script. Allow
>>>>> control of script compilation with a "compile" PI for FXML files.
>>>>>
>>>>> Problem
>>>>> =======
>>>>> javafx.fxml.FXMLLoader is able to execute scripts in Java script languages
>>>>> (javax.script.ScriptEngine implementations) referred to or embedded in a 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 javax.script.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 repetitively invoked and evaluated.
>>>>>
>>>>> Solution
>>>>> ========
>>>>> Before evaluating the script code test whether the javax.script.ScriptEngine implements
>>>>> javax.script.Compilable. If so, compile the script to a javax.script.CompiledScript object first
>>>>> and then use its eval() method to evaluate the script, otherwise continue to use the
>>>>> javax.script.ScriptEngine's eval() method instead. Should compilation of a script yield
>>>>> (unexpectedly) a javax.script.ScriptException then fall back to using the
>>>>> javax.script.ScriptEngine's eval() method. A new process instruction "compile" allows control of
>>>>> the compilation of scripts ("true" sets compilation on, "false" to set compilation off) in FXML
>>>>> files.
>>>>>
>>>>> Specification
>>>>> =============
>>>>> If a javax.script.ScriptEngine implements the javax.script.Compilable interface, then use its
>>>>> compile() method to compile the script to a javax.script.CompiledScript object and use its
>>>>> eval() method to run the script. In the case that the compilation throws (unexpectedly) a
>>>>> javax.script.ScriptException log a warning and fall back to using the
>>>>> javax.script.ScriptEngine's eval() method instead.
>>>>> To allow setting this feature off and on while processing the FXML file a "compile" process
>>>>> instruction ("<?compile false?>" or "<?compile true?>") gets defined that allows to turn
>>>>> compilation off and on throughout a FXML file.
>>>>>
>>>>> Having never seen a real CSR I hope that this matches what is expected and is helpful for
>>>>> assessment. If not please advise (got the name of these fields from [1]).
>>>>>
>>>>> ---
>>>>>
>>>>> Also added brief information about the respective test units (what they test and yield) in the WIP [2].
>>>>>
>>>>> ---rony
>>>>>
>>>>> [1] "CSR-FAQ": <https://wiki.openjdk.java.net/display/csr/CSR+FAQs>
>>>>>
>>>>> [2] "WIP: Script compilable+compile PI+fallback: 8238080: FXMLLoader: if script engines implement
>>>>> javax.script.Compilable compile scripts #192": <https://github.com/openjdk/jfx/pull/192>
>>>>>
>>>>>
>>>>> On 20.04.2020 14:58, Rony G. Flatscher wrote:
>>>>>> There is a new WIP at <https://github.com/openjdk/jfx/pull/192>:
>>>>>>
>>>>>> This WIP adds the ability for a fallback in case compilation of scripts fails, in which case a
>>>>>> warning gets issued about this fact and evaluation of the script will be done without
>>>>>> compilation. Because of the fallback scripts get compiled with this version by default. It
>>>>>> extends PR 187 #187.
>>>>>>
>>>>>> To further ease spotting scripts that cause a ScriptException a message in the form of
>>>>>> "filename: caused ScriptException" gets added to the exception handling in either of the three
>>>>>> locations: an error message, a stack trace or a wrap-up into a RuntimeException (having three
>>>>>> different kinds of reporting ScriptExceptions may be questioned, however none of these tear down
>>>>>> the FXML GUI).
>>>>>>
>>>>>> This WIP comes with proper test units as well. As per Kevin's suggestion a warning gets logged
>>>>>> whenever a script cannot be compiled and the fallback gets used.
>>>>>>
>>>>>> It is suggested to use this WIP as it includes the compilation by default with a safe fallback to
>>>>>> evaluate the uncompiled script, if compilation (unexpectedly) fails.
>>>>>>
>>>>>> Again, any feedback, discussion welcome!
>>>>>>
>>>>>> ---rony
>>>>>>
>>>>>> P.S.: In the log history there is a commit message "Make message more pregnant.", it should have
>>>>>> read "Make messages more terse." instead|.||
>>>>>> |
>>>>>>
>>>>>>
>>>>>> On 17.04.2020 19:37, Rony G. Flatscher wrote:
>>>>>>> There is a new WIP at <https://github.com/openjdk/jfx/pull/187> which adds a compile PI (process
>>>>>>> instruction) for turning on and off script compilation if the script engine implements the
>>>>>>> Compilable interface.
>>>>>>>
>>>>>>> By default compilation is off (no compilation), such that one needs to add a compile PI
>>>>>>> ("<?compile?>") at the top to activate this feature. Supplying "true" (default) or "false" as the PI
>>>>>>> data turns this feature on and off.
>>>>>>>
>>>>>>> The WIP comes with adapted test units that test "compile on" for an entire fxml file, "compile off",
>>>>>>> alternating using "compile on and off", and alternating using "compile off and on". This will test
>>>>>>> all variants of applying the compile PI for all categories of scripts.
>>>>>>>
>>>>>>> Any feedback appreciated!
>>>>>>>
>>>>>>> ---rony
>>>>>>>
>>>>>>> P.S.: FXML files that contain unknown PIs do not cause a runtime error by FXMLLoader, they just get
>>>>>>> ignored. Therefore one could apply the compile PI to FXML files that are used in older JavaFX runtimes.
>>>>>>>
>>>>>>> P.P.S.: In the next days I will also add Kevin's idea in a separate version that will have a
>>>>>>> fallback solution in case a compilation is (unexpectedly) not successful, reverting to
>>>>>>> (interpretative) evaluation/execution of the script. In that version it is planned to have
>>>>>>> compilation on by default as in the case of a compilation failure there will be a safe backup solution.
>>>>>>>
>>>>>>>
>>>>>>> On 14.04.2020 19:52, Kevin Rushforth wrote:
>>>>>>>> Yes, I agree that enough time has gone by. Go ahead with your proposal. I would wait a bit to
>>>>>>>> create the CSR until the review is far enough along to know which direction we intend to go.
>>>>>>>>
>>>>>>>> Unless there is a real concern about possible regressions if scripts are compiled by default, I
>>>>>>>> think "enabled by default" is the way to go. Your argument that such script engines are broken
>>>>>>>> seems reasonable, since this only applies to script engines that implement javax.script.Compilable
>>>>>>>> in the first place. We still might want to add way to turn compilation off for individual scripts.
>>>>>>>> One other thing to consider is that if compilation fails, it might make sense to log a warning and
>>>>>>>> fall back to the existing interpreted mode.
>>>>>>>>
>>>>>>>> Does anyone else have any concerns with this?
>>>>>>>>
>>>>>>>> -- Kevin
>>>>>>>>
>>>>>>>>
>>>>>>>> On 4/14/2020 9:48 AM, Rony G. Flatscher wrote:
>>>>>>>>> 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
>
More information about the openjfx-dev
mailing list