OpenROAD icon indicating copy to clipboard operation
OpenROAD copied to clipboard

gui: remove click+wait forever trap in Timing Report

Open oharboe opened this issue 1 year ago • 3 comments

In small designs a large number of paths can easily be displayed, but for large designs this effectively runs forever.

To avoid this lockup trap, do not save number of timing paths.

This is not ideal, but if the choice is between saving path count and the gui "crashing" for large designs by accidentally clicking "Timing Report Update", then it seems better not to save for now.

Perhaps in the future, for large designs, a progress requester could be made and a "display fetched paths so far" could be added or the performance could be improved to the point that the problem can be ignored.

oharboe avatar Oct 12 '24 07:10 oharboe

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] avatar Oct 12 '24 07:10 github-actions[bot]

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] avatar Oct 12 '24 14:10 github-actions[bot]

clang-tidy review says "All clean, LGTM! :+1:"

github-actions[bot] avatar Oct 15 '24 08:10 github-actions[bot]

@maliberty Ping? Move ahead with this?

oharboe avatar Oct 22 '24 05:10 oharboe

I still don't think this is the right solution. It's a solution that takes away functionality for the perceived benefit of one user with a particular problem (when I ran it on my machine the delay was a few minutes so not too terrible). I'd rather see a warning dialog over removing this.

gadfort avatar Oct 22 '24 13:10 gadfort

What would the warning say? your machine is locked up? The user knows that already.

oharboe avatar Oct 22 '24 14:10 oharboe

This is not a choice between nice alternatives: pick some misbehavior until it can be fixed properly.

oharboe avatar Oct 22 '24 14:10 oharboe

my understanding is that it is a nontrivial amount of work to fix properly(a generic progressbar with a "cancel" capability for long running operations in the GUI), so whatever misbehavior we pick(we have one today), we will be living with it for some time.

oharboe avatar Oct 22 '24 14:10 oharboe

@gadfort already created an option to skip loading the settings. Is that sufficient?

maliberty avatar Oct 22 '24 16:10 maliberty

@gadfort already created an option to skip loading the settings. Is that sufficient?

No, this fix is to avoid a suckerpunch in the first place. If I knew to use that option, I wouldn't click "Update" and lock up the GUI in the first place...

oharboe avatar Oct 22 '24 16:10 oharboe

Let's see if this is solvable in a nicer way - https://github.com/parallaxsw/OpenSTA/issues/111

maliberty avatar Oct 22 '24 17:10 maliberty