Re: RFR: 8274771: Map, FlatMap and OrElse fluent bindings for ObservableValue [v10]
On Thu, 10 Mar 2022 17:49:38 GMT, John Hendrikx wrote: >> This is an implementation of the proposal in >> https://bugs.openjdk.java.net/browse/JDK-8274771 that me and Nir Lisker >> (@nlisker) have been working on. It's a complete implementation including >> good test coverage. >> >> This was based on https://github.com/openjdk/jfx/pull/434 but with a smaller >> API footprint. Compared to the PoC this is lacking public API for >> subscriptions, and does not include `orElseGet` or the `conditionOn` >> conditional mapping. >> >> **Flexible Mappings** >> Map the contents of a property any way you like with `map`, or map nested >> properties with `flatMap`. >> >> **Lazy** >> The bindings created are lazy, which means they are always _invalid_ when >> not themselves observed. This allows for easier garbage collection (once the >> last observer is removed, a chain of bindings will stop observing their >> parents) and less listener management when dealing with nested properties. >> Furthermore, this allows inclusion of such bindings in classes such as >> `Node` without listeners being created when the binding itself is not used >> (this would allow for the inclusion of a `treeShowingProperty` in `Node` >> without creating excessive listeners, see this fix I did in an earlier PR: >> https://github.com/openjdk/jfx/pull/185) >> >> **Null Safe** >> The `map` and `flatMap` methods are skipped, similar to `java.util.Optional` >> when the value they would be mapping is `null`. This makes mapping nested >> properties with `flatMap` trivial as the `null` case does not need to be >> taken into account in a chain like this: >> `node.sceneProperty().flatMap(Scene::windowProperty).flatMap(Window::showingProperty)`. >> Instead a default can be provided with `orElse`. >> >> Some examples: >> >> void mapProperty() { >> // Standard JavaFX: >> label.textProperty().bind(Bindings.createStringBinding(() -> >> text.getValueSafe().toUpperCase(), text)); >> >> // Fluent: much more compact, no need to handle null >> label.textProperty().bind(text.map(String::toUpperCase)); >> } >> >> void calculateCharactersLeft() { >> // Standard JavaFX: >> >> label.textProperty().bind(text.length().negate().add(100).asString().concat(" >> characters left")); >> >> // Fluent: slightly more compact and more clear (no negate needed) >> label.textProperty().bind(text.orElse("").map(v -> 100 - v.length() + >> " characters left")); >> } >> >> void mapNestedValue() { >> // Standard JavaFX: >> label.textProperty().bind(Bindings.createStringBinding( >> () -> employee.get() == null ? "" >> : employee.get().getCompany() == null ? "" >> : employee.get().getCompany().getName(), >> employee >> )); >> >> // Fluent: no need to handle nulls everywhere >> label.textProperty().bind( >> employee.map(Employee::getCompany) >> .map(Company::getName) >> .orElse("") >> ); >> } >> >> void mapNestedProperty() { >> // Standard JavaFX: >> label.textProperty().bind( >> Bindings.when(Bindings.selectBoolean(label.sceneProperty(), >> "window", "showing")) >> .then("Visible") >> .otherwise("Not Visible") >> ); >> >> // Fluent: type safe >> label.textProperty().bind(label.sceneProperty() >> .flatMap(Scene::windowProperty) >> .flatMap(Window::showingProperty) >> .orElse(false) >> .map(showing -> showing ? "Visible" : "Not Visible") >> ); >> } >> >> Note that this is based on ideas in ReactFX and my own experiments in >> https://github.com/hjohn/hs.jfx.eventstream. I've come to the conclusion >> that this is much better directly integrated into JavaFX, and I'm hoping >> this proof of concept will be able to move such an effort forward. > > John Hendrikx has updated the pull request incrementally with one additional > commit since the last revision: > > Process review comments (2) The code looks good. I've left some inline comments. modules/javafx.base/src/main/java/com/sun/javafx/binding/Subscription.java line 40: > 38: * > 39: * For example: > 40: * Subscription s = property.subscribe(System.out::println) I believe our recommended pattern for example code is: {@code ... } modules/javafx.base/src/main/java/com/sun/javafx/binding/Subscription.java line 67: > 65: */ > 66: default Subscription and(Subscription other) { > 67: Objects.requireNonNull(other); This exception could be documented with `@throws NullPointerException if {@code other} is null` modules/javafx.base/src/main/java/javafx/beans/value/FlatMappedBinding.java line 68: > 66: }; > 67: } > 68: } Several files are missing newlines after the last closing brace. Do we enforce this? Also, if there's a newline after the first line of a class d
[jfx17u] RFR: 8203463: [Accessibility, Narrator] NPE in TableView
Reviewed-by: kcr - Commit messages: - 8203463: [Accessibility, Narrator] NPE in TableView Changes: https://git.openjdk.java.net/jfx17u/pull/38/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx17u&pr=38&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8203463 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jfx17u/pull/38.diff Fetch: git fetch https://git.openjdk.java.net/jfx17u pull/38/head:pull/38 PR: https://git.openjdk.java.net/jfx17u/pull/38
[jfx17u] RFR: 8197991: Selecting many items in a TableView is very slow
Co-authored-by: Naohiro Yoshimoto Reviewed-by: kcr, aghaisas - Commit messages: - 8197991: Selecting many items in a TableView is very slow Changes: https://git.openjdk.java.net/jfx17u/pull/39/files Webrev: https://webrevs.openjdk.java.net/?repo=jfx17u&pr=39&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8197991 Stats: 403 lines in 5 files changed: 357 ins; 40 del; 6 mod Patch: https://git.openjdk.java.net/jfx17u/pull/39.diff Fetch: git fetch https://git.openjdk.java.net/jfx17u pull/39/head:pull/39 PR: https://git.openjdk.java.net/jfx17u/pull/39
[jfx11u] Integrated: 8197991: Selecting many items in a TableView is very slow
On Thu, 17 Mar 2022 21:01:51 GMT, Johan Vos wrote: > Co-authored-by: Naohiro Yoshimoto > Reviewed-by: kcr, aghaisas This pull request has now been integrated. Changeset: 1c3d9666 Author:Johan Vos URL: https://git.openjdk.java.net/jfx11u/commit/1c3d9666884b6689d7b982a8541c692468512bf0 Stats: 403 lines in 5 files changed: 357 ins; 40 del; 6 mod 8197991: Selecting many items in a TableView is very slow Backport-of: 8c4f966b163f68c517574e38c394b9349551ad08 - PR: https://git.openjdk.java.net/jfx11u/pull/82
[jfx17u] Integrated: 8203463: [Accessibility, Narrator] NPE in TableView
On Fri, 18 Mar 2022 07:42:11 GMT, Johan Vos wrote: > Reviewed-by: kcr This pull request has now been integrated. Changeset: 53d443bf Author:Johan Vos URL: https://git.openjdk.java.net/jfx17u/commit/53d443bffa34c971c7c6192649caa9aba2acb5f5 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod 8203463: [Accessibility, Narrator] NPE in TableView Backport-of: c705bd493931d88650542be5466d6add359f45b9 - PR: https://git.openjdk.java.net/jfx17u/pull/38
[jfx17u] Integrated: 8197991: Selecting many items in a TableView is very slow
On Fri, 18 Mar 2022 07:42:37 GMT, Johan Vos wrote: > Co-authored-by: Naohiro Yoshimoto > Reviewed-by: kcr, aghaisas This pull request has now been integrated. Changeset: 4af7ee97 Author:Johan Vos URL: https://git.openjdk.java.net/jfx17u/commit/4af7ee97f37cb18b3433afc3b128b10d3401f29c Stats: 403 lines in 5 files changed: 357 ins; 40 del; 6 mod 8197991: Selecting many items in a TableView is very slow Backport-of: 8c4f966b163f68c517574e38c394b9349551ad08 - PR: https://git.openjdk.java.net/jfx17u/pull/39
[jfx11u] Integrated: 8203463: [Accessibility, Narrator] NPE in TableView
On Thu, 17 Mar 2022 20:56:32 GMT, Johan Vos wrote: > Reviewed-by: kcr This pull request has now been integrated. Changeset: bf32dd8a Author:Johan Vos URL: https://git.openjdk.java.net/jfx11u/commit/bf32dd8a52876b0c0a53e9f73d64b0a986829626 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod 8203463: [Accessibility, Narrator] NPE in TableView Backport-of: c705bd493931d88650542be5466d6add359f45b9 - PR: https://git.openjdk.java.net/jfx11u/pull/81
Question about fatal JavaFX crashes
Hello, I take the liberty to ask on the email reflector if there are other people with similar problems. Since updating my application to JavaFX18 I get random fatal crashes. Unfortunately it is not predictable but after some time the app crashes with "EXCEPTION_ACCESS_VIOLATION". The hs_error_pidX reports the following. --- T H R E A D --- Current thread (0x016be9c9b410): JavaThread "C2 CompilerThread0" daemon [_thread_in_native, id=3068, stack(0x00393e80,0x00393e90)] Current CompileTask: C2:10515286 18185 4 javafx.scene.control.TableView$5::onChanged (1049 bytes) Am I the only one seeing this? I am not sure if this relates to changing JavaFX 17 to 18 or whether it makes it just more likely. It seems to be related to "javafx.scene.control.TableView$5::onChanged" since all crashes show this line. Having said that, I cannot provide a test-case since it happens all of a sudden and sometimes after hours using the application. I am open to any feedback or input on how to proceed best. Thanks! -- Daniel
Re: Question about fatal JavaFX crashes
I haven't seen this one; the code in TableView in onChanged hasn't had any updates 6 years. I have my doubts this is a JavaFX problem as it seems the Compiler is crashing here in a piece of native code. It's possible this problem is only occuring on systems where the compiler decides it needs to compile (or recompile) this method, so at a minimum you'd need to make use of TableView in a way to hits that code path often enough to trigger compilation. Oracle provides some documentation on how to deal with what looks like a compiler bug, please read here: https://docs.oracle.com/en/java/javase/17/troubleshoot/troubleshoot-system-crashes.html Further, I think you should report this problem with as much details as possible on bugs.openjdk.java.net -- you can look for similar bugs to see what kind of detail and settings would be helpful to debug this. Other things you could try to solve it yourself: - Run a different Java version - Try switching to a different GC - Use different VM options (are you using anything special?) - Anything else that is not often used, non-standard or experimental, try going to a more common setup --John On 18/03/2022 09:43, Daniel Peintner wrote: Hello, I take the liberty to ask on the email reflector if there are other people with similar problems. Since updating my application to JavaFX18 I get random fatal crashes. Unfortunately it is not predictable but after some time the app crashes with "EXCEPTION_ACCESS_VIOLATION". The hs_error_pidX reports the following. --- T H R E A D --- Current thread (0x016be9c9b410): JavaThread "C2 CompilerThread0" daemon [_thread_in_native, id=3068, stack(0x00393e80,0x00393e90)] Current CompileTask: C2:10515286 18185 4 javafx.scene.control.TableView$5::onChanged (1049 bytes) Am I the only one seeing this? I am not sure if this relates to changing JavaFX 17 to 18 or whether it makes it just more likely. It seems to be related to "javafx.scene.control.TableView$5::onChanged" since all crashes show this line. Having said that, I cannot provide a test-case since it happens all of a sudden and sometimes after hours using the application. I am open to any feedback or input on how to proceed best. Thanks! -- Daniel
Re: RFR: 8274771: Map, FlatMap and OrElse fluent bindings for ObservableValue [v10]
On Thu, 17 Mar 2022 20:09:23 GMT, Michael Strauß wrote: >> John Hendrikx has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Process review comments (2) > > modules/javafx.base/src/main/java/com/sun/javafx/binding/Subscription.java > line 40: > >> 38: * >> 39: * For example: >> 40: * Subscription s = property.subscribe(System.out::println) > > I believe our recommended pattern for example code is: > > {@code > ... > } Fixed this everywhere. > modules/javafx.base/src/main/java/com/sun/javafx/binding/Subscription.java > line 67: > >> 65: */ >> 66: default Subscription and(Subscription other) { >> 67: Objects.requireNonNull(other); > > This exception could be documented with `@throws NullPointerException if > {@code other} is null` I've updated the docs a bit -- it hasn't received much attention because this is not going to be API for now > modules/javafx.base/src/main/java/javafx/beans/value/FlatMappedBinding.java > line 68: > >> 66: }; >> 67: } >> 68: } > > Several files are missing newlines after the last closing brace. Do we > enforce this? > > Also, if there's a newline after the first line of a class declaration, > shouldn't there also be a newline before the last closing brace? Let me add those new lines at the end of files (everywhere) as Github is also flagging it with an ugly red marker. I tend to unconsciously remove them myself on longer files as it looks weird in editors to have an unused line at the bottom. As for the newline before the last closing brace, that doesn't seem to be done a lot in the current code base. I've added those newlines at the top as it seems fairly consistent in the code base, although I'm not a fan as I use empty lines only to separate things when there is no clear separation already (like an opening brace). > modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java > line 146: > >> 144: * Creates an {@code ObservableValue} that holds the result of >> applying a >> 145: * mapping on this {@code ObservableValue}'s value. The result is >> updated >> 146: * when this {@code ObservableValue}'s value changes. If this value >> is > > I think a lot of the new documentation in this class sacrifices > understandability for precision in trying to deal with the difference between > "this ObservableValue" and "this ObservableValue's value". > > However, my feeling is that that's not helping users who are trying to > understand the purpose of the new APIs. > What do you think about a simplified version like this: > `Creates a new {@ObservableValue} that applies a mapping function to this > {@code ObservableValue}. The result is updated when this {@code > ObservableValue} changes.` > > Sure, it's not literally mapping _this ObservableValue instance_, but would > this language really confuse readers more that the precise language? > > Another option might be to combine both: > `Creates a new {@ObservableValue} that applies a mapping function to this > {@code ObservableValue}. More precisely, it creates a new {@code > ObservableValue} that holds the result of applying a mapping function to the > value of this {@code ObservableValue}.` Yeah, agreed, it is a bit annoying to have to deal with the fact that these classes are wrappers around an actual value and having to refer to them as such to be "precise". I'm willing to make another pass at all of these to change the wording. What do you think @nlisker ? > modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java > line 153: > >> 151: * >> 152: * var text = new SimpleStringProperty("abcd"); >> 153: * ObservableValueupperCase = >> text.map(String::toUpperCase); > > Escaping `<` and `>` is not necessary if the code block is wrapped in > `{@code}` Also fixed everywhere > modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java > line 197: > >> 195: /** >> 196: * Creates an {@code ObservableValue} that holds the value of an >> {@code ObservableValue} >> 197: * resulting from applying a mapping on this {@code >> ObservableValue}'s value. The result > > While technically correct, I think the first sentence should focus more on > the purpose of this method. > > How about something like this: > `Creates a new {@code ObservableValue} that holds the value of a nested > {@code ObservableValue} by applying a mapping function to extract the nested > {@code ObservableValue}.` > > That's not as precise, but it makes the purpose much more clear. I've changed this to use your wording as I think it does read much better. Perhaps also possible: Creates a new {@code ObservableValue} that holds the value of a nested {@code ObservableValue} supplied by the given mapping function. ? > modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java > line 227: > >>
Re: RFR: 8274771: Map, FlatMap and OrElse fluent bindings for ObservableValue [v11]
> This is an implementation of the proposal in > https://bugs.openjdk.java.net/browse/JDK-8274771 that me and Nir Lisker > (@nlisker) have been working on. It's a complete implementation including > good test coverage. > > This was based on https://github.com/openjdk/jfx/pull/434 but with a smaller > API footprint. Compared to the PoC this is lacking public API for > subscriptions, and does not include `orElseGet` or the `conditionOn` > conditional mapping. > > **Flexible Mappings** > Map the contents of a property any way you like with `map`, or map nested > properties with `flatMap`. > > **Lazy** > The bindings created are lazy, which means they are always _invalid_ when not > themselves observed. This allows for easier garbage collection (once the last > observer is removed, a chain of bindings will stop observing their parents) > and less listener management when dealing with nested properties. > Furthermore, this allows inclusion of such bindings in classes such as `Node` > without listeners being created when the binding itself is not used (this > would allow for the inclusion of a `treeShowingProperty` in `Node` without > creating excessive listeners, see this fix I did in an earlier PR: > https://github.com/openjdk/jfx/pull/185) > > **Null Safe** > The `map` and `flatMap` methods are skipped, similar to `java.util.Optional` > when the value they would be mapping is `null`. This makes mapping nested > properties with `flatMap` trivial as the `null` case does not need to be > taken into account in a chain like this: > `node.sceneProperty().flatMap(Scene::windowProperty).flatMap(Window::showingProperty)`. > Instead a default can be provided with `orElse`. > > Some examples: > > void mapProperty() { > // Standard JavaFX: > label.textProperty().bind(Bindings.createStringBinding(() -> > text.getValueSafe().toUpperCase(), text)); > > // Fluent: much more compact, no need to handle null > label.textProperty().bind(text.map(String::toUpperCase)); > } > > void calculateCharactersLeft() { > // Standard JavaFX: > > label.textProperty().bind(text.length().negate().add(100).asString().concat(" > characters left")); > > // Fluent: slightly more compact and more clear (no negate needed) > label.textProperty().bind(text.orElse("").map(v -> 100 - v.length() + " > characters left")); > } > > void mapNestedValue() { > // Standard JavaFX: > label.textProperty().bind(Bindings.createStringBinding( > () -> employee.get() == null ? "" > : employee.get().getCompany() == null ? "" > : employee.get().getCompany().getName(), > employee > )); > > // Fluent: no need to handle nulls everywhere > label.textProperty().bind( > employee.map(Employee::getCompany) > .map(Company::getName) > .orElse("") > ); > } > > void mapNestedProperty() { > // Standard JavaFX: > label.textProperty().bind( > Bindings.when(Bindings.selectBoolean(label.sceneProperty(), "window", > "showing")) > .then("Visible") > .otherwise("Not Visible") > ); > > // Fluent: type safe > label.textProperty().bind(label.sceneProperty() > .flatMap(Scene::windowProperty) > .flatMap(Window::showingProperty) > .orElse(false) > .map(showing -> showing ? "Visible" : "Not Visible") > ); > } > > Note that this is based on ideas in ReactFX and my own experiments in > https://github.com/hjohn/hs.jfx.eventstream. I've come to the conclusion > that this is much better directly integrated into JavaFX, and I'm hoping this > proof of concept will be able to move such an effort forward. John Hendrikx has updated the pull request incrementally with five additional commits since the last revision: - Reword flat map docs a bit and fixed a link - Add missing javadoc tags - Clean up docs in Subscription - Fix code blocks - Add line feed at last line where it was missing - Changes: - all: https://git.openjdk.java.net/jfx/pull/675/files - new: https://git.openjdk.java.net/jfx/pull/675/files/8f9bf897..6a5358d7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jfx&pr=675&range=10 - incr: https://webrevs.openjdk.java.net/?repo=jfx&pr=675&range=09-10 Stats: 39 lines in 7 files changed: 11 ins; 0 del; 28 mod Patch: https://git.openjdk.java.net/jfx/pull/675.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/675/head:pull/675 PR: https://git.openjdk.java.net/jfx/pull/675
Re: RFR: 8274771: Map, FlatMap and OrElse fluent bindings for ObservableValue [v11]
On Fri, 18 Mar 2022 10:32:36 GMT, John Hendrikx wrote: >> This is an implementation of the proposal in >> https://bugs.openjdk.java.net/browse/JDK-8274771 that me and Nir Lisker >> (@nlisker) have been working on. It's a complete implementation including >> good test coverage. >> >> This was based on https://github.com/openjdk/jfx/pull/434 but with a smaller >> API footprint. Compared to the PoC this is lacking public API for >> subscriptions, and does not include `orElseGet` or the `conditionOn` >> conditional mapping. >> >> **Flexible Mappings** >> Map the contents of a property any way you like with `map`, or map nested >> properties with `flatMap`. >> >> **Lazy** >> The bindings created are lazy, which means they are always _invalid_ when >> not themselves observed. This allows for easier garbage collection (once the >> last observer is removed, a chain of bindings will stop observing their >> parents) and less listener management when dealing with nested properties. >> Furthermore, this allows inclusion of such bindings in classes such as >> `Node` without listeners being created when the binding itself is not used >> (this would allow for the inclusion of a `treeShowingProperty` in `Node` >> without creating excessive listeners, see this fix I did in an earlier PR: >> https://github.com/openjdk/jfx/pull/185) >> >> **Null Safe** >> The `map` and `flatMap` methods are skipped, similar to `java.util.Optional` >> when the value they would be mapping is `null`. This makes mapping nested >> properties with `flatMap` trivial as the `null` case does not need to be >> taken into account in a chain like this: >> `node.sceneProperty().flatMap(Scene::windowProperty).flatMap(Window::showingProperty)`. >> Instead a default can be provided with `orElse`. >> >> Some examples: >> >> void mapProperty() { >> // Standard JavaFX: >> label.textProperty().bind(Bindings.createStringBinding(() -> >> text.getValueSafe().toUpperCase(), text)); >> >> // Fluent: much more compact, no need to handle null >> label.textProperty().bind(text.map(String::toUpperCase)); >> } >> >> void calculateCharactersLeft() { >> // Standard JavaFX: >> >> label.textProperty().bind(text.length().negate().add(100).asString().concat(" >> characters left")); >> >> // Fluent: slightly more compact and more clear (no negate needed) >> label.textProperty().bind(text.orElse("").map(v -> 100 - v.length() + >> " characters left")); >> } >> >> void mapNestedValue() { >> // Standard JavaFX: >> label.textProperty().bind(Bindings.createStringBinding( >> () -> employee.get() == null ? "" >> : employee.get().getCompany() == null ? "" >> : employee.get().getCompany().getName(), >> employee >> )); >> >> // Fluent: no need to handle nulls everywhere >> label.textProperty().bind( >> employee.map(Employee::getCompany) >> .map(Company::getName) >> .orElse("") >> ); >> } >> >> void mapNestedProperty() { >> // Standard JavaFX: >> label.textProperty().bind( >> Bindings.when(Bindings.selectBoolean(label.sceneProperty(), >> "window", "showing")) >> .then("Visible") >> .otherwise("Not Visible") >> ); >> >> // Fluent: type safe >> label.textProperty().bind(label.sceneProperty() >> .flatMap(Scene::windowProperty) >> .flatMap(Window::showingProperty) >> .orElse(false) >> .map(showing -> showing ? "Visible" : "Not Visible") >> ); >> } >> >> Note that this is based on ideas in ReactFX and my own experiments in >> https://github.com/hjohn/hs.jfx.eventstream. I've come to the conclusion >> that this is much better directly integrated into JavaFX, and I'm hoping >> this proof of concept will be able to move such an effort forward. > > John Hendrikx has updated the pull request incrementally with five additional > commits since the last revision: > > - Reword flat map docs a bit and fixed a link > - Add missing javadoc tags > - Clean up docs in Subscription > - Fix code blocks > - Add line feed at last line where it was missing modules/javafx.base/src/main/java/com/sun/javafx/binding/Subscription.java line 57: > 55: * Cancels this subscription. > 56: */ > 57: void unsubscribe(); to me `unsubscribe` feels not natural, I'm not unsubscribing a `Subscription` but I'm eg disposing it, hence to me `dispose` would feel better but I'm not a native speaker. Another pattern I see often is that I store a subscription in a field and recreate it under certain conditions: private Subscription mySubscription; void refresh() { if( this.mySubscription != null ) { this.mySubscription.dispos(); } this.mySubscription = } if Subscription would provide ```java static void dispose(Subscription subscription) { if( su
Re: RFR: 8274771: Map, FlatMap and OrElse fluent bindings for ObservableValue [v10]
On Fri, 18 Mar 2022 09:48:39 GMT, John Hendrikx wrote: >> modules/javafx.base/src/main/java/com/sun/javafx/binding/Subscription.java >> line 67: >> >>> 65: */ >>> 66: default Subscription and(Subscription other) { >>> 67: Objects.requireNonNull(other); >> >> This exception could be documented with `@throws NullPointerException if >> {@code other} is null` > > I've updated the docs a bit -- it hasn't received much attention because this > is not going to be API for now Yes, in "phase 2" when this class is made public there will be a proper docs review. - PR: https://git.openjdk.java.net/jfx/pull/675
Re: RFR: 8274771: Map, FlatMap and OrElse fluent bindings for ObservableValue [v10]
On Fri, 18 Mar 2022 09:32:18 GMT, John Hendrikx wrote: >> modules/javafx.base/src/main/java/javafx/beans/value/FlatMappedBinding.java >> line 68: >> >>> 66: }; >>> 67: } >>> 68: } >> >> Several files are missing newlines after the last closing brace. Do we >> enforce this? >> >> Also, if there's a newline after the first line of a class declaration, >> shouldn't there also be a newline before the last closing brace? > > Let me add those new lines at the end of files (everywhere) as Github is also > flagging it with an ugly red marker. I tend to unconsciously remove them > myself on longer files as it looks weird in editors to have an unused line at > the bottom. > > As for the newline before the last closing brace, that doesn't seem to be > done a lot in the current code base. I've added those newlines at the top as > it seems fairly consistent in the code base, although I'm not a fan as I use > empty lines only to separate things when there is no clear separation already > (like an opening brace). I don't think jcheck checks for newlines anywhere. Usually the style that I see is a newline after the definition of the class and at the end of the file (sometimes), but not before the last closing brace. - PR: https://git.openjdk.java.net/jfx/pull/675
Re: Question about fatal JavaFX crashes
Hi John, all, Thanks for your detailed reply. I submitted a bug report with detailed information. - Run a different Java version > I tried different versions and vendors with the same result. * OpenJDK 17.0.1 * Zulu 17.0.2 * I wanted to check also JDK18-ea but gradle does not yet support it > - Try switching to a different GC > - Use different VM options (are you using anything special?) > - Anything else that is not often used, non-standard or experimental, > try going to a more common setup > I use no specific setting, all is default. I have been reading that the -client flag might help in some cases but unfortunately this flag is no longer taken into account for 64bit JVMS. Thanks, -- Daniel > > --John > > On 18/03/2022 09:43, Daniel Peintner wrote: > > Hello, > > > > I take the liberty to ask on the email reflector if there are other > people > > with similar problems. > > > > Since updating my application to JavaFX18 I get random fatal crashes. > > Unfortunately it is not predictable but after some time the app crashes > > with "EXCEPTION_ACCESS_VIOLATION". > > > > The hs_error_pidX reports the following. > > > > > > --- T H R E A D --- > > > > Current thread (0x016be9c9b410): JavaThread "C2 CompilerThread0" > > daemon [_thread_in_native, id=3068, > > stack(0x00393e80,0x00393e90)] > > > > Current CompileTask: > > C2:10515286 18185 4 > javafx.scene.control.TableView$5::onChanged > > (1049 bytes) > > > > > > Am I the only one seeing this? > > I am not sure if this relates to changing JavaFX 17 to 18 or whether it > > makes it just more likely. > > > > It seems to be related to "javafx.scene.control.TableView$5::onChanged" > > since all crashes show this line. > > > > Having said that, I cannot provide a test-case since it happens all of a > > sudden and sometimes after hours using the application. > > > > I am open to any feedback or input on how to proceed best. > > > > Thanks! > > > > -- Daniel >
Re: Question about fatal JavaFX crashes
I think it is probable that this is a hotspot VM problem in the C2 JIT compiler code. I've moved your bug report to hotspot : https://bugs.openjdk.java.net/browse/JDK-8283386 The interesting question isn't about which version of FX used to work. It is what was the last working version of the JDK. It looks a bit like a JDK 17 bug from the evidence so far so if you were running FX 17 on JDK 17 GA you maybe have picked up a later update release of JDK 17 along with FX 18 ?? Regardless this doesn't look like an FX bug. But unless you can actually provide a test case, or, by luck the hotspot folks recognise the issue, I don't know what can/will happen. -phil. On 3/18/22 7:08 AM, Daniel Peintner wrote: Hi John, all, Thanks for your detailed reply. I submitted a bug report with detailed information. - Run a different Java version I tried different versions and vendors with the same result. * OpenJDK 17.0.1 * Zulu 17.0.2 * I wanted to check also JDK18-ea but gradle does not yet support it - Try switching to a different GC - Use different VM options (are you using anything special?) - Anything else that is not often used, non-standard or experimental, try going to a more common setup I use no specific setting, all is default. I have been reading that the -client flag might help in some cases but unfortunately this flag is no longer taken into account for 64bit JVMS. Thanks, -- Daniel --John On 18/03/2022 09:43, Daniel Peintner wrote: Hello, I take the liberty to ask on the email reflector if there are other people with similar problems. Since updating my application to JavaFX18 I get random fatal crashes. Unfortunately it is not predictable but after some time the app crashes with "EXCEPTION_ACCESS_VIOLATION". The hs_error_pidX reports the following. --- T H R E A D --- Current thread (0x016be9c9b410): JavaThread "C2 CompilerThread0" daemon [_thread_in_native, id=3068, stack(0x00393e80,0x00393e90)] Current CompileTask: C2:10515286 18185 4 javafx.scene.control.TableView$5::onChanged (1049 bytes) Am I the only one seeing this? I am not sure if this relates to changing JavaFX 17 to 18 or whether it makes it just more likely. It seems to be related to "javafx.scene.control.TableView$5::onChanged" since all crashes show this line. Having said that, I cannot provide a test-case since it happens all of a sudden and sometimes after hours using the application. I am open to any feedback or input on how to proceed best. Thanks! -- Daniel
Re: Question about fatal JavaFX crashes
Thanks, Phil. That was my take as well. I don't see how this can be a JavaFX bug given where it is crashing. FWIW, I haven't ever seen anything like this. -- Kevin On 3/18/2022 10:25 AM, Philip Race wrote: I think it is probable that this is a hotspot VM problem in the C2 JIT compiler code. I've moved your bug report to hotspot : https://bugs.openjdk.java.net/browse/JDK-8283386 The interesting question isn't about which version of FX used to work. It is what was the last working version of the JDK. It looks a bit like a JDK 17 bug from the evidence so far so if you were running FX 17 on JDK 17 GA you maybe have picked up a later update release of JDK 17 along with FX 18 ?? Regardless this doesn't look like an FX bug. But unless you can actually provide a test case, or, by luck the hotspot folks recognise the issue, I don't know what can/will happen. -phil. On 3/18/22 7:08 AM, Daniel Peintner wrote: Hi John, all, Thanks for your detailed reply. I submitted a bug report with detailed information. - Run a different Java version I tried different versions and vendors with the same result. * OpenJDK 17.0.1 * Zulu 17.0.2 * I wanted to check also JDK18-ea but gradle does not yet support it - Try switching to a different GC - Use different VM options (are you using anything special?) - Anything else that is not often used, non-standard or experimental, try going to a more common setup I use no specific setting, all is default. I have been reading that the -client flag might help in some cases but unfortunately this flag is no longer taken into account for 64bit JVMS. Thanks, -- Daniel --John On 18/03/2022 09:43, Daniel Peintner wrote: Hello, I take the liberty to ask on the email reflector if there are other people with similar problems. Since updating my application to JavaFX18 I get random fatal crashes. Unfortunately it is not predictable but after some time the app crashes with "EXCEPTION_ACCESS_VIOLATION". The hs_error_pidX reports the following. --- T H R E A D --- Current thread (0x016be9c9b410): JavaThread "C2 CompilerThread0" daemon [_thread_in_native, id=3068, stack(0x00393e80,0x00393e90)] Current CompileTask: C2:10515286 18185 4 javafx.scene.control.TableView$5::onChanged (1049 bytes) Am I the only one seeing this? I am not sure if this relates to changing JavaFX 17 to 18 or whether it makes it just more likely. It seems to be related to "javafx.scene.control.TableView$5::onChanged" since all crashes show this line. Having said that, I cannot provide a test-case since it happens all of a sudden and sometimes after hours using the application. I am open to any feedback or input on how to proceed best. Thanks! -- Daniel
Re: Question about fatal JavaFX crashes
I have at least seen JIT compiler crashes like this in other unrelated cases .. in theory you can use an option like -XX:CompileCommand=exclude,javafx/scene/control/TableView$5::onChanged although I am very unsure about the syntax for the last part of it especially with what looks like some anonymous method or lambda expression. I can almost guarantee that my example is wrong. Excluding the whole of TableView might be easier to specify but will slow you down a lot. -phil. On 3/18/22 10:37 AM, Kevin Rushforth wrote: Thanks, Phil. That was my take as well. I don't see how this can be a JavaFX bug given where it is crashing. FWIW, I haven't ever seen anything like this. -- Kevin On 3/18/2022 10:25 AM, Philip Race wrote: I think it is probable that this is a hotspot VM problem in the C2 JIT compiler code. I've moved your bug report to hotspot : https://bugs.openjdk.java.net/browse/JDK-8283386 The interesting question isn't about which version of FX used to work. It is what was the last working version of the JDK. It looks a bit like a JDK 17 bug from the evidence so far so if you were running FX 17 on JDK 17 GA you maybe have picked up a later update release of JDK 17 along with FX 18 ?? Regardless this doesn't look like an FX bug. But unless you can actually provide a test case, or, by luck the hotspot folks recognise the issue, I don't know what can/will happen. -phil. On 3/18/22 7:08 AM, Daniel Peintner wrote: Hi John, all, Thanks for your detailed reply. I submitted a bug report with detailed information. - Run a different Java version I tried different versions and vendors with the same result. * OpenJDK 17.0.1 * Zulu 17.0.2 * I wanted to check also JDK18-ea but gradle does not yet support it - Try switching to a different GC - Use different VM options (are you using anything special?) - Anything else that is not often used, non-standard or experimental, try going to a more common setup I use no specific setting, all is default. I have been reading that the -client flag might help in some cases but unfortunately this flag is no longer taken into account for 64bit JVMS. Thanks, -- Daniel --John On 18/03/2022 09:43, Daniel Peintner wrote: Hello, I take the liberty to ask on the email reflector if there are other people with similar problems. Since updating my application to JavaFX18 I get random fatal crashes. Unfortunately it is not predictable but after some time the app crashes with "EXCEPTION_ACCESS_VIOLATION". The hs_error_pidX reports the following. --- T H R E A D --- Current thread (0x016be9c9b410): JavaThread "C2 CompilerThread0" daemon [_thread_in_native, id=3068, stack(0x00393e80,0x00393e90)] Current CompileTask: C2:10515286 18185 4 javafx.scene.control.TableView$5::onChanged (1049 bytes) Am I the only one seeing this? I am not sure if this relates to changing JavaFX 17 to 18 or whether it makes it just more likely. It seems to be related to "javafx.scene.control.TableView$5::onChanged" since all crashes show this line. Having said that, I cannot provide a test-case since it happens all of a sudden and sometimes after hours using the application. I am open to any feedback or input on how to proceed best. Thanks! -- Daniel
Aw: Re: ArrayIndexOutOfBoundsException when disconnecting screen
Sorry for the delay. I tested a bit around, unfortunately this bug doesn't happen all the time. It looks like it can also happen when disconnecting just one screen. I have filed a ticket: https://bugs.openjdk.java.net/browse/JDK-8283401 Also one time the JVM crashed, leaving a hs_err_pid file, which I attached as well. -- Marius Gesendet: Mittwoch, 23. Februar 2022 um 23:54 Uhr Von: "Kevin Rushforth" An: "Marius Hanl" , "openjfx-dev" Betreff: Re: ArrayIndexOutOfBoundsException when disconnecting screen I spent a fair bit of time simulating and testing the D3DERR_DEVICEREMOVED cases for RDP disconnect to fix JDK-8239589 [1], but detaching a screen, which will change the number of active adapters, is a somewhat different case (I'm not sure how easy it would be to simulate it). It's very likely that it isn't handled in a robust manner. Do you know whether you need to disconnect 2 screens to reproduce this, or would disconnecting a single external monitor cause it? Based on the stack trace, it looks like the D3DResourceFactory array has been recreated with the reduced number of adapters (1), but the old screen is still being used by the instance of PPSRenderer. Can you file a bug with the stack trace, and as much information about how to reproduce it as you can. -- Kevin [1] [1]https://bugs.openjdk.java.net/browse/JDK-8239589 On 2/23/2022 3:35 AM, Marius Hanl wrote: > I get an ArrayIndexOutOfBoundsException sometimes when I disconnect my > screen(s). > Setup: I have a laptop with a docking station attached. The docking > station is connected to two screen, so when I disconnect it 2 screen > will be 'lost'. > > The following stacktrace is visible and the application is completely > black and unresponsive: > java.lang.ArrayIndexOutOfBoundsException: Index 1 out of bounds for > length 1 > at > com.sun.prism.d3d.D3DPipeline.getD3DResourceFactory(D3DPipeline.java:21 > 7) > at > com.sun.prism.d3d.D3DPipeline.getResourceFactory(D3DPipeline.java:284) > at > com.sun.scenario.effect.impl.prism.ps.PPSRenderer.validate(PPSRenderer. > java:101) > at > com.sun.scenario.effect.impl.prism.ps.PPSRenderer.getCompatibleImage(PP > SRenderer.java:221) > at > com.sun.scenario.effect.impl.prism.ps.PPSRenderer.getCompatibleImage(PP > SRenderer.java:67) > at > com.sun.scenario.effect.Effect.getCompatibleImage(Effect.java:479) > at > com.sun.javafx.sg.prism.NodeEffectInput.getImageDataForBoundedNode(Node > EffectInput.java:228) > at > com.sun.javafx.sg.prism.NodeEffectInput.filter(NodeEffectInput.java:131 > ) > at > com.sun.scenario.effect.FilterEffect.filter(FilterEffect.java:185) > at com.sun.scenario.effect.Offset.filter(Offset.java:160) > at com.sun.scenario.effect.Merge.filter(Merge.java:148) > at > com.sun.scenario.effect.DelegateEffect.filter(DelegateEffect.java:70) > at > com.sun.scenario.effect.impl.prism.PrEffectHelper.render(PrEffectHelper > .java:166) > at > com.sun.javafx.sg.prism.EffectFilter.render(EffectFilter.java:61) > at com.sun.javafx.sg.prism.NGNode.renderEffect(NGNode.java:2384) > at com.sun.javafx.sg.prism.NGNode.doRender(NGNode.java:2069) > at com.sun.javafx.sg.prism.NGNode.render(NGNode.java:1964) > at com.sun.javafx.sg.prism.NGGroup.renderContent(NGGroup.java:270) > at > com.sun.javafx.sg.prism.NGRegion.renderContent(NGRegion.java:579) > at com.sun.javafx.sg.prism.NGNode.doRender(NGNode.java:2072) > ... ( a lot of doRender(..), renderContent(..) calls... ) > at > com.sun.javafx.tk.quantum.ViewPainter.doPaint(ViewPainter.java:480) > at > com.sun.javafx.tk.quantum.ViewPainter.paintImpl(ViewPainter.java:329) > at > com.sun.javafx.tk.quantum.PresentingPainter.run(PresentingPainter.java: > 92) > at > java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors > .java:515) > at > java.base/java.util.concurrent.FutureTask.runAndReset$$$capture(FutureT > ask.java:305) > at > java.base/java.util.concurrent.FutureTask.runAndReset(FutureTask.java) > at com.sun.javafx.tk.RenderJob.run(RenderJob.java:58) > at > java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolE > xecutor.java:1128) > at > java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPool > Executor.java:628) > at > com.sun.javafx.tk.quantum.QuantumRenderer$PipelineRunnable.run(QuantumR > enderer.java:126) > at java.base/java.lang.Thread.run(Thread.java:829) > > --- > --- > > Now my question: > A naive fix would be an array check here but does someone has more > information about the code here? > And more
Re: RFR: 8283346: Optimize observable ArrayList creation in FXCollections
On Thu, 17 Mar 2022 21:10:14 GMT, Marius Hanl wrote: > This simple PR optimizes the observable `ArrayList` creation by using the > ArrayList constructor/array size so that the underlying array will be > initialized at the correct size which will speed up the creation as the array > does not need to grow as a result of the `addAll` call. > > I also added tests which will succeed before and after to verify that nothing > got broken by this change. > Also I made a benchmark test. Results: > > | Benchmark | Mode| Cnt | Score | Error | Units > | - | - | - | - | > - | - | > | ListBenchmark OLD | thrpt | 25 | 722,842 | ± 26,93 | ops/s > | ListBenchmark NEW | thrpt | 25 | 29262,274 | ± 2088,712 | ops/s > > Benchmark code > > > import javafx.collections.FXCollections; > import javafx.collections.ObservableList; > import org.openjdk.jmh.annotations.Benchmark; > import org.openjdk.jmh.annotations.Scope; > import org.openjdk.jmh.annotations.Setup; > import org.openjdk.jmh.annotations.State; > import org.openjdk.jmh.annotations.TearDown; > > import java.util.ArrayList; > import java.util.List; > > @State(Scope.Benchmark) > public class ListBenchmark { > > List strings; > > @Setup > public void setup() { > strings = new ArrayList<>(); > for(int i = 0; i< 10;i++) { > strings.add("abc: " + i); > } > } > > @TearDown > public void tearDown() { > strings = null; > } > > @Benchmark > public ObservableList init() { > return FXCollections.observableArrayList(strings); > } > } > > > This PR appears to have two internal behavior changes. A. Initial collection size (PR owner's description) B. By adding the item directly to the backingList Avoiding beginChange () --doAdd () --endChange () ``` java @Override protected void doAdd (int index, E element) { if (elementObserver! = null) elementObserver.attachListener (element); backingList.add (index, element); } Looking at the following code, it seems that there is no problem because attachListener () is called in the constructor. We will need to make sure that the current unit tests cover B's changes. ObservableListWrapper.java ``` java public ObservableListWrapper(List list) { backingList = list; elementObserver = null; } public ObservableListWrapper(List list, Callback extractor) { backingList = list; this.elementObserver = new ElementObserver(extractor, new Callback() { @Override public InvalidationListener call(final E e) { return new InvalidationListener() { @Override public void invalidated(Observable observable) { beginChange(); int i = 0; final int size = size(); for (; i < size; ++i) { if (get(i) == e) { nextUpdate(i); } } endChange(); } }; } }, this); final int sz = backingList.size(); for (int i = 0; i < sz; ++i) { elementObserver.attachListener(backingList.get(i)); } } - PR: https://git.openjdk.java.net/jfx/pull/758
Re: Promote addEventHandler/removeEventHandler to EventTarget interface
On 17/03/2022 21:01, Michael Strauß wrote: I'm working on an application that uses the JavaFX event system extensively, and I'm finding it quite hard to use common code for event handler registrations. The problem is that the `EventTarget` interface contains no addEventHandler/removeEventHandler methods, and as a consequence of that, code that uses `EventTarget` ends up requiring lots of brittle instanceof tests to call these methods on all the different implementations of `EventTarget`. There are three buckets of `EventTarget` implementations: 1) Implementations that declare the following methods: void addEventHandler(EventType, EventHandler); void removeEventHandler(EventType, EventHandler); void addEventFilter(EventType, EventHandler); void removeEventFilter(EventType, EventHandler); --> Node, Scene, Window, Transform, Task, Service 2) Implementations that declare the following methods: void addEventHandler(EventType, EventHandler); void removeEventHandler(EventType, EventHandler); --> MenuItem, TreeItem, TableColumnBase (Note that the EventHandler argument ist parameterized as EventHandler, not EventHandler as in the first set of methods.) 3) Implementations that don't declare any methods to add or remove event handlers: --> Dialog, Tab I think the situation can be improved by promoting the bucket 1 methods to the `EventTarget` interface, so they can be used consistently across all implementations of `EventTarget`. This works seamlessly for bucket 1 and bucket 3 implementations. I think it would make good sense that they also have the EventHandler/Filters methods for consistency. If they're targets for Events, then you should be able to add handlers or filters for them. With bucket 2, there's the problem that, inconsistently, the EventHandler argument is not a lower-bounded wildcard. Unfortunately, a method with an EventHandler parameter cannot implement an interface method that expects EventHandler. However, since the erasure of the method remains the same, changing the method signature would technically be a binary-compatible change. They also forward directly to the internal EventHandlerManager which does accept EventHandler. This seems like an oversight when those implementations were added to MenuItem, TreeItem and TableColumnBase. Do you think this is a useful improvement? I think that all the implementations should definitely be consistent; Dialog and Tab probably should have the means to add EventHandlers and EventFilters; and MenuItem, TreeItem, TableColumnBase could do with the EventFilter variants. If they're going to be consistent, might as well add these methods to the EventTarget interface to enforce this for any future EventTarget implementations. EventTarget is already public API, and so there might be 3rd party implementations. This means that the methods added to the EventTarget interface must be default methods. It would be super if these default implementations would just work out of the box, but that would require exposing the internal class EventHandlerManager (and adding a `getEventHandlerManager` to the `EventTarget` interface). If that's not realistic, then initially the default implementations would have to throw UnsupportedOperationException. --John
Re: Question about fatal JavaFX crashes
I wonder, could you do the opposite and force compilation to trigger the bug more consistently? Scott > On Mar 18, 2022, at 2:03 PM, Philip Race wrote: > > I have at least seen JIT compiler crashes like this in other unrelated > cases .. > > in theory you can use an option like > > -XX:CompileCommand=exclude,javafx/scene/control/TableView$5::onChanged > > although I am very unsure about the syntax for the last part of it especially > with what looks like > some anonymous method or lambda expression. I can almost guarantee that my > example is wrong. > > Excluding the whole of TableView might be easier to specify but will slow you > down a lot. > > -phil. > > >> On 3/18/22 10:37 AM, Kevin Rushforth wrote: >> Thanks, Phil. That was my take as well. I don't see how this can be a JavaFX >> bug given where it is crashing. FWIW, I haven't ever seen anything like this. >> >> -- Kevin >> >> >>> On 3/18/2022 10:25 AM, Philip Race wrote: >>> I think it is probable that this is a hotspot VM problem in the C2 JIT >>> compiler code. >>> >>> I've moved your bug report to hotspot : >>> https://bugs.openjdk.java.net/browse/JDK-8283386 >>> The interesting question isn't about which version of FX used to work. >>> It is what was the last working version of the JDK. >>> It looks a bit like a JDK 17 bug from the evidence so far so if >>> you were running FX 17 on JDK 17 GA you maybe have picked up a later update >>> release of JDK 17 along with FX 18 ?? >>> >>> Regardless this doesn't look like an FX bug. >>> But unless you can actually provide a test case, or, by luck the hotspot >>> folks recognise the issue, >>> I don't know what can/will happen. >>> >>> -phil. >>> >>> On 3/18/22 7:08 AM, Daniel Peintner wrote: Hi John, all, Thanks for your detailed reply. I submitted a bug report with detailed information. - Run a different Java version I tried different versions and vendors with the same result. * OpenJDK 17.0.1 * Zulu 17.0.2 * I wanted to check also JDK18-ea but gradle does not yet support it > - Try switching to a different GC > - Use different VM options (are you using anything special?) > - Anything else that is not often used, non-standard or experimental, > try going to a more common setup > I use no specific setting, all is default. I have been reading that the -client flag might help in some cases but unfortunately this flag is no longer taken into account for 64bit JVMS. Thanks, -- Daniel > --John > > On 18/03/2022 09:43, Daniel Peintner wrote: >> Hello, >> >> I take the liberty to ask on the email reflector if there are other > people >> with similar problems. >> >> Since updating my application to JavaFX18 I get random fatal crashes. >> Unfortunately it is not predictable but after some time the app crashes >> with "EXCEPTION_ACCESS_VIOLATION". >> >> The hs_error_pidX reports the following. >> >> >> --- T H R E A D --- >> >> Current thread (0x016be9c9b410): JavaThread "C2 CompilerThread0" >> daemon [_thread_in_native, id=3068, >> stack(0x00393e80,0x00393e90)] >> >> Current CompileTask: >> C2:10515286 18185 4 > javafx.scene.control.TableView$5::onChanged >> (1049 bytes) >> >> >> Am I the only one seeing this? >> I am not sure if this relates to changing JavaFX 17 to 18 or whether it >> makes it just more likely. >> >> It seems to be related to "javafx.scene.control.TableView$5::onChanged" >> since all crashes show this line. >> >> Having said that, I cannot provide a test-case since it happens all of a >> sudden and sometimes after hours using the application. >> >> I am open to any feedback or input on how to proceed best. >> >> Thanks! >> >> -- Daniel >>> >> >
Re: RFR: 8274771: Map, FlatMap and OrElse fluent bindings for ObservableValue [v10]
On Fri, 18 Mar 2022 10:17:01 GMT, John Hendrikx wrote: >> modules/javafx.base/src/main/java/javafx/beans/value/ObservableValue.java >> line 197: >> >>> 195: /** >>> 196: * Creates an {@code ObservableValue} that holds the value of an >>> {@code ObservableValue} >>> 197: * resulting from applying a mapping on this {@code >>> ObservableValue}'s value. The result >> >> While technically correct, I think the first sentence should focus more on >> the purpose of this method. >> >> How about something like this: >> `Creates a new {@code ObservableValue} that holds the value of a nested >> {@code ObservableValue} by applying a mapping function to extract the nested >> {@code ObservableValue}.` >> >> That's not as precise, but it makes the purpose much more clear. > > I've changed this to use your wording as I think it does read much better. > > Perhaps also possible: > > Creates a new {@code ObservableValue} that holds the value of a nested > {@code ObservableValue} supplied > by the given mapping function. > > ? Both seem fine, I don't have any preference over one or the other. - PR: https://git.openjdk.java.net/jfx/pull/675
Re: RFR: 8283063: Optimize Observable{List/Set/Map}Wrapper.retainAll/removeAll
On Sat, 12 Mar 2022 04:57:37 GMT, Michael Strauß wrote: > `Observable{List/Set/Map}Wrapper.retainAll/removeAll` can be optimized for > some edge cases. > > 1. `removeAll(c)`: > This is a no-op if 'c' is empty. > For `ObservableListWrapper`, returning early skips an object allocation. For > `ObservableSetWrapper` and `ObservableMapWrapper`, returning early prevents > an enumeration of the entire collection. > > 2. `retainAll(c)`: > This is a no-op if the backing collection is empty, or equivalent to > `clear()` if `c` is empty. > > I've added some tests to verify the optimized behavior for each of the three > classes. This is an equivalent change that partially includes the fix I proposed in #305. It is good to apply it to Map / Set. - PR: https://git.openjdk.java.net/jfx/pull/751
Re: RFR: 8283346: Optimize observable ArrayList creation in FXCollections
On Thu, 17 Mar 2022 21:10:14 GMT, Marius Hanl wrote: > This simple PR optimizes the observable `ArrayList` creation by using the > ArrayList constructor/array size so that the underlying array will be > initialized at the correct size which will speed up the creation as the array > does not need to grow as a result of the `addAll` call. > > I also added tests which will succeed before and after to verify that nothing > got broken by this change. > Also I made a benchmark test. Results: > > | Benchmark | Mode| Cnt | Score | Error | Units > | - | - | - | - | > - | - | > | ListBenchmark OLD | thrpt | 25 | 722,842 | ± 26,93 | ops/s > | ListBenchmark NEW | thrpt | 25 | 29262,274 | ± 2088,712 | ops/s > > Benchmark code > > > import javafx.collections.FXCollections; > import javafx.collections.ObservableList; > import org.openjdk.jmh.annotations.Benchmark; > import org.openjdk.jmh.annotations.Scope; > import org.openjdk.jmh.annotations.Setup; > import org.openjdk.jmh.annotations.State; > import org.openjdk.jmh.annotations.TearDown; > > import java.util.ArrayList; > import java.util.List; > > @State(Scope.Benchmark) > public class ListBenchmark { > > List strings; > > @Setup > public void setup() { > strings = new ArrayList<>(); > for(int i = 0; i< 10;i++) { > strings.add("abc: " + i); > } > } > > @TearDown > public void tearDown() { > strings = null; > } > > @Benchmark > public ObservableList init() { > return FXCollections.observableArrayList(strings); > } > } > > > The following pieces of code should be identical: var list = FXCollections.observableArrayList(); list.addAll(source); var list = FXCollections.observableList(new ArrayList<>(source)); Any observable difference in behavior would be unspecified. When it comes to `modCount`, that's an internal field of `AbstractList`. `FXCollections.observableArrayList()` and `FXCollections.observableList(...)` are not the right place to test this. - PR: https://git.openjdk.java.net/jfx/pull/758