#5620 closed defect (fixed)
Moving diagrams in NF too slow
Reported by: | massimo ceraolo | Owned by: | Adrian Pop |
---|---|---|---|
Priority: | blocker | Milestone: | 1.18.0 |
Component: | Interactive Environment | Version: | |
Keywords: | Cc: | Martin Sjölund, Adeel Asghar |
Description (last modified by )
Slowness in performing graphical operations on model diagrams in OMEdit has always been a weakness of OM.
Although I cannot provide a test, after several days of intensive usage, I think the situation has worsened with NF.
In this ticket, I concentrate on moving objects (and object sets)
Just as an example try this:
- duplicate Electrical-analog-examples-CauerLowPassOPV
- select everything
- click arrow-left
Moving the model representation left takes a huge time: 15s on my laptop! It is near-instantaneous on the same PC in Dymola.
This is very bad. Consider that one usually moves by keyboard instead of mouse-dragging to be more accurate in positioning, but this requires several keystrokes in a row.
There are other merely graphical operations that are too slow for a satisfying programmer's life, but for me, this is the first to be reported.
Attachments (2)
Change History (38)
comment:1 by , 5 years ago
comment:2 by , 5 years ago
Priority: | critical → blocker |
---|
comment:3 by , 5 years ago
To be more precise, I repeated time measuring starting from a freshly rebooted PC, and just-opened OMEdit. This way repeating this ticket's description procedure I measured 22s. This larger time is probably due to the absence of any OMEdit caching but changes nothing about the ticket meaning.
I'll enclose omeditcommunications.log.
I don't know what's happening, but I dare to say that there should be specific APIs for cosmetic operations such as moving, to avoid the more sophisticate OMC actions that occur when "meaningful" (i.e. non-just cosmetic) operations occur.
by , 5 years ago
Attachment: | omeditcommunication.log added |
---|
comment:4 by , 5 years ago
Description: | modified (diff) |
---|
comment:5 by , 5 years ago
Component: | *unknown* → NF API |
---|---|
Milestone: | 2.0.0 → 1.14.0 |
Owner: | changed from | to
Status: | new → assigned |
Summary: | Graphical operations probably slower with NF - moving objects case → Moving diagrams is slow because of updateConnection() API call |
I checked the log. As I understand it, the culprit is the updateConnection()
API call, which is called to update the graphical annotation, and takes between 70 and 180 ms each time it gets called. Since there are many connections in the diagram, the overall cumulated time is around 15 s. What is interesting is that the updateComponent()
API call, which is used to update the component annotations, takes 1 ms or less.
@adrpo, can you please check why the updatedConnection()
call is so much slower than updateComponent()
and fix it using the NF?
follow-up: 9 comment:6 by , 5 years ago
Is there a way to disable nfAPI?
Since I could not find this way, I uninstalled OM on the same PC of previous tests, re-installed rel v1.14.0-dev-234, and repeated the test in comment 3.
Result: the same operation took around 1s instead of 22s (obviously with the OF).
comment:7 by , 5 years ago
Summary: | Moving diagrams is slow because of updateConnection() API call → Moving diagrams in NF much slower than in OF because of updateConnection() API call |
---|
comment:8 by , 5 years ago
You can disable the nfAPI with -d=-nfAPI in the settings I guess.
I will have a look why it takes so much more time with the NF, I'm not even sure the NF is used here.
follow-up: 10 comment:9 by , 5 years ago
Replying to ceraolo:
Is there a way to disable nfAPI?
Since I could not find this way, I uninstalled OM on the same PC of previous tests, re-installed rel v1.14.0-dev-234, and repeated the test in comment 3.
Result: the same operation took around 1s instead of 22s (obviously with the OF).
Uncheck Tools->Options->Simulation->Enable new frontend use in the OMC API (faster GUI response)
, save and restart OMEdit.
comment:10 by , 5 years ago
Uncheck
Tools->Options->Simulation->Enable new frontend use in the OMC API (faster GUI response)
, save and restart OMEdit.
Ok. It seems to me that moving with keys does not depend on having the NF or OF in place.
Therefore, I made new tests comparing OM 1.13.2 (in an Oracle Virtual box) and OM 1.14.0-dev26697, with new frontend API disabled according to comment 9 instructions.
The tests still involve moving Electrical.analog.examples.CauerLowPassOPV through arrow keys on the keyboard.
The tests made today are not comparable with the ones in comment 6, because they were made on a different (and faster) PC.
Today's results:
- using OM 1.13.2, a sequence of 5 movements by keyboard (5 fast clicks on an arrow key) took 1.2s
- using OM 1.14.0-dev26697, sequence of 5 movements by keyboard (5 fast clicks on an arrow key) took 13 s
So I found one order of magnitude of speed ratio between the two OM versions on this specific test.
The situation seems improved over comment 6 (there the 1s measure was unreliable, maybe it was a few tens of s), but still the newer OM version is much slower than the older.
And this seems to be unrelated to the choice of front end.
Maybe this operation is always still performed by the OF?
comment:11 by , 5 years ago
Milestone: | 1.14.0 → 1.15.0 |
---|
Releasing 1.14.0 which is stable and has many improvements w.r.t. 1.13.2.
This issue, previously marked as blocker for 1.14.0, is rescheduled to 1.15.0
comment:12 by , 5 years ago
An update on this ticket
the situation is now worse than ever. I repeated the test in comment 10.
I.e. move with 5 fast clicks Electrical-analog-examples-CauerLowPassOPV
The result is:
OM 1.13.2: 1.5s
OM 1.16-dev 256 66s (yes around a 50 times worsening!)
I really think that something very bad has occurred in this direction in recent months and this ticket (or at least #5653) should really get top priority.
follow-up: 14 comment:13 by , 5 years ago
On my PC with with 1.16.0-dev-330 it is not that bad, it takes about one second for each system-wide motion.
Anyway, as soon as the PR with replaceable support is finally merged in, we'll look into this.
What I see from the omeditcommunication.log file is that each updateConnection
takes on average 100 ms, which is a lot of time considering that you just need to add an offset to all the extents. Add up several dozens of connections (which is not unusual), and that becomes unbearably slow.
@adrpo, @perost, I guess one of you should have a look into it. Maybe @perost could do that, since @adrpo is busy with the PR merging?
comment:14 by , 5 years ago
Replying to casella:
On my PC with 1.16.0-dev-330 it is not that bad, it takes about one second for each system-wide motion.
Do you mean first select-all then click on an arrow? Your PC is like a lightning!
I tried dev-330 on my PC and got still 6.5 s for a single click to move the whole system.
My PC is not very fast, but not too old: i5-8400 @2.8GHz.
On such a PC I expect that moving a diagram should take a split second (in Dymola I cannot measure this time, so fast it is).
comment:15 by , 4 years ago
Milestone: | 1.15.0 → 1.16.0 |
---|
Release 1.15.0 was scrapped, because replaceable support eventually turned out to be more easily implemented in 1.16.0. Hence, all 1.15.0 tickets are rescheduled to 1.16.0
comment:16 by , 4 years ago
@adrpo reports:
Currently we go through all the equations and for each connection we transform the connectors into strings which we check against. This happens even if we already
found the connect we need to update and we updated it.
I guess this could be easily optimized with a reasonable amount of work.
comment:19 by , 4 years ago
Cc: | added |
---|---|
Component: | NF API → Interactive Environment |
I found out what the problem is. Moving the API for updateComponent to typed API via PR:
https://github.com/OpenModelica/OMCompiler/pull/3061
has slowed it down drastically and it also depends on the size of the AST now. Typing the calls using the old front-end is what is taking so much time.
comment:20 by , 4 years ago
This should properly fix it:
https://github.com/OpenModelica/OpenModelica/pull/6764
comment:22 by , 4 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
follow-up: 24 comment:23 by , 4 years ago
I checked on my desktop.
A five-keystroke sequence took 2.8s. On a previous version (v1.16.0 dev 330) the same sequence takes 30s. On an older version, before the appearance of OF (1.14-dev 234), the time spent for the same sequence on the same PC is 5s
So, we roughly have now slightly more than a half the time we used to have two years ago, and less than one tenth of what we had yesterday.
Good result!
Since NF was expected to be much faster than OF maybe there is still room for further optimisation, but this is just a hypothesis of mine.
comment:24 by , 4 years ago
Replying to ceraolo:
I checked on my desktop.
A five-keystroke sequence took 2.8s.
On my PC we are below 2 s.
Since NF was expected to be much faster than OF maybe there is still room for further optimisation, but this is just a hypothesis of mine.
Should we investigate more for the forthcoming versions of the tool?
comment:26 by , 4 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
It turned out that the example I proposed in this ticket captures only one case of slowness when moving.
After the fix in comment 21 the example in this ticket description improved a lot.
However, in other cases moving objects is still a real pain. One such case is when we move an object ofn a model that belongs to a multi-model package, like the attacked invPWM.
Steps to reproduce:
- load the package invPWM
- open model invPWM.ThreePhase.Id3PwmAronMod
- select Rload
- click on right arrow 5 times in a row to move that resistor towards right. This operation on my laptop takes 11.5s, by far unacceptable. Consider that in Dymola the same move is instantaneous.
by , 4 years ago
comment:27 by , 4 years ago
Summary: | Moving diagrams in NF much slower than in OF because of updateConnection() API call → Moving diagrams in NF too slow |
---|
follow-up: 29 comment:28 by , 4 years ago
OK, this is one of those cases that @adeas31 was mentioning to me. On my pc, each move takes 1.5 s, which is mostly spent running diffModelicaFileListings()
. This is due to the fact that the entire package is contained in a single 224 kB file, which needs to be processed every time some change is applied to the source code (e.g. when a component is moved).
The easy workaround is to split the package in several files, which will speed up diffModelicaFileListings()
.
The easiest solution from the point of view of OMEdit develpment would probably be to speed up diffModelicaFileListings()
, I'm not sure how much margin we have there.
Another possibility is to wait until the user has stopped doing stuff on the diagram for some time, e.g. 2 seconds, and only apply diffModelicaFileListings()
at that point on the cumulated actions. I understand that would make all the cumulated actions an atomic operation w.r.t. the undo action, but I guess this is a minor nuisance compared to the current slugginess.
@adeas, @sjoelund.se, what do you think?
follow-up: 30 comment:29 by , 4 years ago
The easy workaround is to split the package in several files, which will speed up
diffModelicaFileListings()
.
The easiest solution from the point of view of OMEdit development would probably be to speed up
diffModelicaFileListings()
, I'm not sure how much margin we have there.
It is strange that splitting into several files makes things faster. Maybe to use diffModelicaFileListings(), that I imagine searches entire files, is not the right tool to support moving individual objects? I mentioned that dymola is instantaneous; maybe because its operation is unrelated to the file size?
From a user point of view, a 224 kB file is rather a small file. Avoiding splitting it gives advantages in terms of management on HD of archives composed of several packages. I have a package per teaching topic on my course, and splitting is possible but non-optimal for me.
Maybe from OM point-of-view, a 224 kB file is large, but this is another story. In principle, tools should not force the user's choices, unless the latter are unreasonable.
comment:30 by , 4 years ago
Replying to ceraolo:
It is strange that splitting into several files makes things faster. Maybe to use diffModelicaFileListings(), that I imagine searches entire files, is not the right tool to support moving individual objects?
Every time you change something (anything) in a model, including graphical annotations, the entire representation of the file it is stored into is updated, even if the file is not saved.
I mentioned that dymola is instantaneous; maybe because its operation is unrelated to> the file size?
Dymola probably has a different architecture, which btw is not public.
From a user point of view, a 224 kB file is rather a small file.
I agree, and in fact I'm not happy about this situation myself.
Avoiding splitting it gives advantages in terms of management on HD of archives composed of several packages. I have a package per teaching topic on my course, and splitting is possible but non-optimal for me.
Maybe from OM point-of-view, a 224 kB file is large, but this is another story. In principle, tools should not force the user's choices, unless the latter are unreasonable.
I totally agree. If the specification allows to do something, we should support it unconditionally.
comment:33 by , 4 years ago
See 0e0a981/OpenModelica. The commit tries to defer the model updating code and gives a lot better user experience. Note that the actual delay caused by the diff api is still there but is only done once at the end.
comment:34 by , 4 years ago
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
I just tried with the latest nightly, the problems reported in the original description and in comment:26 are solved, the reaction to arrow keystrokes on my PC are virtually instantaneous.
I'm going to close this ticket. @ceraolo, trac is currently disabled because of the on-going migration of tickets to GitHub. If you still see problems with this, please write to me and I'll reopen the ticket.
Thanks!
comment:35 by , 4 years ago
@ceraolo, when testing please make sure you Options | General | Enable new frontend use in the OMC API (faster GUI response)
enabled.
comment:36 by , 4 years ago
I checked.
Excellent result!
With this ticket fixed the user experience improves very much.
@ceraolo, can you please help us pinpointing the root cause:
Thanks!