On Wed, 12 Feb 2025 17:39:57 GMT, Andy Goryachev <ango...@openjdk.org> wrote:

>> Root Cause:
>> (Multiple) properties are getting bound to the global 
>> `Platform.accessibilityActive` property. Binding (and I say, accessing) of 
>> properties is not thread-safe.
>> 
>> I also changed the design a bit.  Originally, every symbol in a chart had 
>> its `focusTraversableProperty` bound to `Platform.accessibilityActive` in 
>> order to enable the accessibility navigation across the chart data points.  
>> This is rather inefficient, as the property has to manage (thousands?) of 
>> listeners.
>> 
>> Instead, a single boolean property is added to each chart, with a listener 
>> added to it which iterates over data symbols to toggle the 
>> `focusTraversableProperty` directly.
>> 
>> The exact moment when the new property gets bound is also important, and has 
>> to happen in the FX application thread.
>> 
>> With this change, it is safe to create and populate charts with data in a 
>> background thread.
>> 
>> ---
>> 
>> ## NOTES
>> 
>> 1. It looks like the `Platform.accessibilityActive` property never 
>> transitions back to false  after it transitioned to true.  Some say it is 
>> because there is no mechanism in the platform to get notified which cannot 
>> possibly be true.
>> 2. The javadoc for `Platform.accessibilityActiveProperty()` method 
>> stipulates that "_This method may be called from any thread_" which is 
>> patently not true as the current implementation is simply not thread-safe.
>
> Andy Goryachev has updated the pull request with a new target base due to a 
> merge or a rebase. The pull request now contains 21 commits:
> 
>  - enabled pie chart test
>  - Merge branch 'master' into 8349091.charts.thread.safety
>  - Merge branch 'master' into 8349091.charts.thread.safety
>  - whitespace
>  - Merge remote-tracking branch 'origin/master' into 
> 8349091.charts.thread.safety
>  - cleanup
>  - tests pass
>  - chart tests only
>  - more jitter
>  - Merge remote-tracking branch 'origin/master' into 
> 8348423.node.thread.safety
>  - ... and 11 more: https://git.openjdk.org/jfx/compare/1e691573...93fc7b93

Sorry, forgot to enable the pie chart test.
On a positive note, I've tested the last commit in the cumulative branch
https://github.com/andy-goryachev-oracle/jfx/tree/ag.thread.safe.init
which includes all the fixes, the heap size lowered from 1g to 128m, and the 
test duration increased 3x to 15 seconds.

-------------

PR Comment: https://git.openjdk.org/jfx/pull/1697#issuecomment-2654425798

Reply via email to