A new thread is spawned for every reconciled transaction?

[kkoning]

Hi!

When using the reconcile dialogue on an account with ~1500 transactions (after an import), the released version (JavaFX interface) hung, and needed to be force quit. Manually increasing the maximum java heap size appears to have fixed that issue, but this task still took several minutes of >100% cpu use. After curiosity as to what could be taking so long, firing up JVisualVM showed that jgnash had >1000 threads running. This number sank over the course of a few minutes, and eventually went back down to a reasonable number once the processing was complete.

I was using a version built from the git sources downloaded tonight (2/22/17) and built with IntelliJ. Running on OSX 10.12. Also, thanks for writing this app! =D

[lukebakken]

Might be related to #21, maybe not.

[kkoning]

I thought it might be, but in that thread you had linked to a commit that was supposed to have fixed the issue, and I had downloaded and used the git HEAD just a few minutes before. I was hoping it might be something relatively obvious related to the design, but if it's something you'd have to dig for, I'll roll up my sleeves and send you a patch. :)

[lukebakken]

You're right, I missed the part where you built from source. Hopefully you or @ccavanaugh can find a fix. I see the same symptoms when reconciling items.

[kkoning]

Fixed; submitted a pull request here: https://github.com/ccavanaugh/jgnash/pull/24

[ccavanaugh]

Kendall, Thanks for finding and reporting the problem and I think you've found the root of the problem.

I've stress tested your patch by performing a large bulk deletion of the transactions forcing packTable to be called. At least on Linux, it's still able to bring the whole desktop to a complete stall for several moments. I'm thinking root cause is buried deeper within Java if it's able to freeze the desktop. Within TableViewManager I have a rate limiting executor (updateColumnVisibilityExecutor) that simply dumps excess events ensuring the last one is run. I think a similar approach can be taken by ignoring the packTable requests in-between and ensuring only the last one is executed.This prevents wasted CPU cycles and should prevent freezing.

[ccavanaugh]

I should have mention that the complete desktop freeze was present before and after the patch.

I've committed a trial that improves the freezing significantly at my end, but I would be curious to see how / if it helps on OSX.

Thanks!

[kkoning]

You're welcome.

It did seem like the function of packTable() was to resize UI elements, which doesn't seem necessary to do after each individual update--both generally, and because the particular change here is to a column that's of fixed width. But, I'll admit, I just did a rather quick look through the code based on where all the threads were starting, as it let me fix the symptoms with the least amount of impact on the rest of the code. Your solution sounds superior, since you can be more confident we're not breaking something else. :)

It freeze up the entire desktop, i.e., not just the app itself? Even before my fix, it didn't have that effect on MacOS. I wonder if it might be something specific to linux scheduling?

Also, I tried the same delete test with removing a lot of transactions. It took quite some time, because, since the recalculation of table size is a function of the number of items in the table, the delete operation is O^2 of the number of transactions. Your solution of reducing the number of times that this recalculation is done is much better there.

Looking at your commit, it seems that packTable() submits a future task to the application's event queue, so, even rate-limiting it to 1 thread (for the submission) might still create a lot of those events. The documentation for Platform.runlater looks like it discourages the practice of submitting large numbers of these events, and encourages batching. Even if those function calls are rate limited, it might still submit a few hundred events to the application's event queue, which would explain the unresponsiveness.

Ideally, it would be nice to perform all the data model updates as a single operation, and just update the user interface once after that action has completed. But I'm not as familiar with GUI programming (or the way jgnash is structured) to know whether that's feasible.

[ccavanaugh]

So... this has been fun. The recommend use of Platform.runlater changed from JavaFX 2 to 8 and I missed that. Thanks for pointing it out!

I've implemented a version of Platform.runlater that batches together Runnables based on background execution time. I've seeing anywhere from 8 runnables being batched to as high as 1250 for some operations. I'm seeing noticeable improvements in many areas now.

I still see room to rate limit some additional register table updates involved in transaction changes which will further improve performance of large updates.

[ccavanaugh]

Closed, fixed in release 2.29.0