Opened 5 years ago

Closed 3 years ago

Last modified 3 years ago

#5620 closed defect (fixed)

Moving diagrams in NF too slow

Reported by: ceraolo Owned by: adrpo
Priority: blocker Milestone: 1.18.0
Component: Interactive Environment Version:
Keywords: Cc: sjoelund.se, adeas31

Description (last modified by ceraolo)

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)

omeditcommunication.log (3.3 MB) - added by ceraolo 5 years ago.
InvPWM.mo (224.5 KB) - added by ceraolo 4 years ago.

Change History (38)

comment:1 Changed 5 years ago by casella

@ceraolo, can you please help us pinpointing the root cause:

  • perform the operation at some exact known time, based on your PC clock
  • open the omeditcommunication.log file and look for operations carried out at that time
  • try to pinpoint the API call(s) that took the larger amounts of time and report them

Thanks!

comment:2 Changed 5 years ago by casella

  • Priority changed from critical to blocker

comment:3 Changed 5 years ago by ceraolo

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.

Last edited 5 years ago by ceraolo (previous) (diff)

Changed 5 years ago by ceraolo

comment:4 Changed 5 years ago by ceraolo

  • Description modified (diff)

comment:5 Changed 5 years ago by casella

  • Component changed from *unknown* to NF API
  • Milestone changed from 2.0.0 to 1.14.0
  • Owner changed from somebody to adrpo
  • Status changed from new to assigned
  • Summary changed from Graphical operations probably slower with NF - moving objects case to 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?

comment:6 follow-up: Changed 5 years ago by 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).

Last edited 5 years ago by ceraolo (previous) (diff)

comment:7 Changed 5 years ago by ceraolo

  • Summary changed from Moving diagrams is slow because of updateConnection() API call to Moving diagrams in NF much slower than in OF because of updateConnection() API call

comment:8 Changed 5 years ago by adrpo

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.

comment:9 in reply to: ↑ 6 ; follow-up: Changed 5 years ago by adeas31

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 in reply to: ↑ 9 Changed 5 years ago by ceraolo

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 Changed 5 years ago by casella

  • Milestone changed from 1.14.0 to 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 Changed 4 years ago by ceraolo

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.

Last edited 4 years ago by ceraolo (previous) (diff)

comment:13 follow-up: Changed 4 years ago by casella

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 in reply to: ↑ 13 Changed 4 years ago by ceraolo

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 Changed 4 years ago by casella

  • Milestone changed from 1.15.0 to 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 Changed 4 years ago by casella

@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.

https://github.com/OpenModelica/OpenModelica/blob/master/OMCompiler/Compiler/Script/InteractiveUtil.mo#L16177

I guess this could be easily optimized with a reasonable amount of work.

comment:17 Changed 4 years ago by adrpo

I already did the modifications, will push it in asap.

comment:19 Changed 4 years ago by adrpo

  • Cc sjoelund.se adeas31 added
  • Component changed from NF API to 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:21 Changed 4 years ago by casella

Thank you Adrian!

I hope the PR passes the test, we'll try it tomorrow.

comment:22 Changed 4 years ago by adrpo

  • Resolution set to fixed
  • Status changed from assigned to closed

comment:23 follow-up: Changed 4 years ago by ceraolo

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.

Last edited 4 years ago by ceraolo (previous) (diff)

comment:24 in reply to: ↑ 23 Changed 4 years ago by casella

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:25 Changed 4 years ago by adrpo

There are ways to make this faster, we will do them for 1.17.

comment:26 Changed 4 years ago by ceraolo

  • Resolution fixed deleted
  • Status changed from closed to 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.
Last edited 4 years ago by ceraolo (previous) (diff)

Changed 4 years ago by ceraolo

comment:27 Changed 4 years ago by ceraolo

  • Summary changed from Moving diagrams in NF much slower than in OF because of updateConnection() API call to Moving diagrams in NF too slow

comment:28 follow-up: Changed 4 years ago by casella

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?

comment:29 in reply to: ↑ 28 ; follow-up: Changed 4 years ago by ceraolo

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.

Last edited 4 years ago by ceraolo (previous) (diff)

comment:30 in reply to: ↑ 29 Changed 4 years ago by casella

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:31 Changed 4 years ago by casella

  • Milestone changed from 1.16.0 to 1.17.0

Retargeted to 1.17.0 after 1.16.0 release

comment:32 Changed 4 years ago by casella

  • Milestone changed from 1.17.0 to 1.18.0

Rescheduled to 1.18.0

comment:33 Changed 3 years ago by adeas31

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.

Last edited 3 years ago by adeas31 (previous) (diff)

comment:34 Changed 3 years ago by casella

  • Resolution set to fixed
  • Status changed from reopened to 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 Changed 3 years ago by casella

@ceraolo, when testing please make sure you Options | General | Enable new frontend use in the OMC API (faster GUI response) enabled.

comment:36 Changed 3 years ago by ceraolo

I checked.
Excellent result!
With this ticket fixed the user experience improves very much.

Note: See TracTickets for help on using tickets.