Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix endless update loop in the presence of empty DataSet #674

Merged
merged 2 commits into from
Nov 20, 2024

Conversation

pacher
Copy link
Contributor

@pacher pacher commented Aug 21, 2024

Describe the bug
Empty DataSet causes some kind of endless update loop somewhere.

To Reproduce
Create chart, add two empty datasets. Wait a second (to let JFX thread do pending tasks). After a second add a point to one of the datasets.

My reproducer in Kotlin

    val chart = frame("ChartFx", 1366.0 * 0.7, 768.0) {
        val xAxis = DefaultNumericAxis("x")
        val yAxis = DefaultNumericAxis("y")
        XYChart(xAxis, yAxis)
    }

    val first = DoubleDataSet("First")
    val second = DoubleDataSet("Second")

    withContext(Dispatchers.JavaFx) {
        chart.datasets.addAll(first, second)
    }

    delay(1000)
    first.add(150.0, 25.0)

Observed behavior
High CPU and GPU usage, exceptions like

java.util.concurrent.RejectedExecutionException: Task com.sun.javafx.tk.quantum.PaintRenderJob@3195e9e1[Not completed, task = java.util.concurrent.Executors$RunnableAdapter@75ae38c1[Wrapped task = com.sun.javafx.tk.quantum.PresentingPainter@1ed15372]] rejected from com.sun.javafx.tk.quantum.QuantumRenderer@643b1d11[Terminated, pool size = 0, active threads = 0, queued tasks = 0, completed tasks = 4074]
	at java.base/java.util.concurrent.ThreadPoolExecutor$AbortPolicy.rejectedExecution(ThreadPoolExecutor.java:2081)
	at java.base/java.util.concurrent.ThreadPoolExecutor.reject(ThreadPoolExecutor.java:841)

when trying to exit application.
Also endless loop (and the fix) was confirmed by a println statement inside updateAxisRange function.
The problem goes away as soon as a point added to the second empty dataset.

Fix
Suggested simple fix solves the problem.

Copy link

sonarcloud bot commented Aug 21, 2024

Copy link

sonarcloud bot commented Sep 30, 2024

Copy link
Member

@wirew0rm wirew0rm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for reporting your findings and this fix.
The early return in the updateAxisRange call is a good call in any way, although one could argue that in any way it should not be possible to create an endless loop this way.
I assume there are still ways to trigger the original problem (e.g. a dataset with one Double.Inf) will not do the early return but probably still endlessly retrigger the computation as in this case there will be no valid updated range.
But as this check does not cause any unneeded cognitive and computational complexity and solves a real problem (or at the least makes it way less probable), I'll merge this and than the underlying problem can be resolved later on.

I also ran the code formatting on the change.

@wirew0rm wirew0rm merged commit 8d15ad2 into fair-acc:main Nov 20, 2024
5 of 6 checks passed
@pacher pacher deleted the fix-endless-loop branch November 20, 2024 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants