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

Project loading-time in RC2.8 significantly longer than 1.1.3 #3312

Closed
musikBear opened this issue Jan 31, 2017 · 17 comments
Closed

Project loading-time in RC2.8 significantly longer than 1.1.3 #3312

musikBear opened this issue Jan 31, 2017 · 17 comments

Comments

@musikBear
Copy link

musikBear commented Jan 31, 2017

OS : winXP sp3
LMMS 1.1.90-RC2.8
2 gb ram

I have a project i use as sketchbook. It is 35 mins long.
This project earlier loaded in ~ 50 secd
In RC2.8 this takes ~4.5mins
It looks like lmms uses progressively longer time for longer files
I have made a file over SkiesiOnions that shows this
The file is expanded to 11 mins
Compare the loading of the default SkiesiOnions and the the 11 mins 'version'
On my system the loading halts around 50% and from there it takes ~3 mins to load the project.
This is different from earlier versions of lmms, and i am sure RC2.2 loaded files faster
Is this also happening on better pc's?
A proper benchmark for loading one specific file would perhaps be a good thing to establish, so this feature could be less 'ad hoc' and instead precisely determined on one specified system

File: https://lmms.io/lsp/?action=show&file=10232

@michaelgregorius
Copy link
Contributor

I can confirm that loading got significantly slower between 1.1.3 and the current master (c8e9873). Here are some manual measurements that have been acquired with a stop watch while loading https://lmms.io/lsp/?action=show&file=10232:
1.1.3: ~4.5s
master: ~9.0s

Some profiling with valgrind shows that most of the delta is spent in MemoryPool::getChunks(int). This method in turn is mainly called by Note::createDetuning().

For a visual overview here is the call graph for 1.1.3:
valgrind - version 1 1 3

And here is the call graph for master:
valgrind - master

The problem was also described in #2029.

@michaelgregorius
Copy link
Contributor

Commenting out the macro MM_OPERATORS in DetuningHelper also does not help. Loading the file still takes ~9s in this case. So the cost is in fact caused by the sheer number of DetuningHelper instances that are created.

@musikBear
Copy link
Author

There is a connection to the new theme
If i choose the Classic theme, i can load the file above in 1.05 min (stop-watch)
@michaelgregorius Can you measure difference in loading of the file above with Classic versus Default theme, and out of curiosity, what are your loading-time(s)?

@michaelgregorius
Copy link
Contributor

When I switch master to the classic theme it still takes ~9s on my machine. The profiler results also do not indicate that there is a problem with the theme. The problem is caused by the DetuningHelpers which have been introduced somewhere between 1.1.3 and master.

@PhysSong
Copy link
Member

I think this is because of the our inefficient memory manager. MemoryPool::getChunks() uses very bad algorithm. It really needs improvements.

@zonkmachine
Copy link
Member

With the new memory manager the loading speed is something like 2x faster for 'Onion' and 'Krem Kaakkuja'.

@lukas-w
Copy link
Member

lukas-w commented Oct 30, 2017

👍 I think we should still keep this open until the DetuningHelper problem is resolved.

@michaelgregorius
Copy link
Contributor

Opening the project https://lmms.io/lsp/?action=show&file=10232 now takes 7 seconds (stop watch) using a release with debug info build of commit 68c85c8.

A fresh valgrind run indicates that the problem with the DetuningHelper seems to be gone. The opening use case is now dominated by calls to Song::updateLength() which in turn calls Track::length() very often:
3312-tracklength
I assume that too many small updates take place while the file is loading. It would likely suffice to only call Song::updateLength() once at the end when the whole song is loaded.

Here's the full callgrind output:
callgrind-TrackLength.zip

@michaelgregorius
Copy link
Contributor

michaelgregorius commented Oct 30, 2017

For some further motivation I have put a return statement at the very top of Song::updateLength to see how this affects the the time it takes to load the test file. It only takes 2 seconds in that case and the GUI state at the end of loading does not even look wrong (although this is definitively not a final fix).

Here's the call graph of that build:
3312-songupdatelengthimmediatereturn
It looks rather "noisy" as is expected for code with no big bottleneck.

Full callgrind output:
callgrind-ImmediateReturn.zip

@musikBear
Copy link
Author

I would like to add an observation about windows states.
If i save a very large 'project' (~40 mins -its a scratch-book with ideas) with song-editor open, then i can forget about opening that project again. I will have to convert to mmp, and change state 1 to 0 for 'open', then i can re-open the project. I have had the 'open-state' wait for 60 mins, but it does not open.
There is a peculiarity, because the flag 0 for open, does not mean that the song-editor is closed when it is being populated. The result of the 0-flag, will close song-editor after all the tracks has been loaded, but during loading, song-editor is visible.
I have made it a habit to close all open windows, before i final-save any project, in order to avoid this glitch, -is ofcause hw related, but very real,( witch was why i protested against forced opening of windows)
Most likely noone with more ram than 3 gb will ever have this issue, at all

@tresf
Copy link
Member

tresf commented Oct 30, 2017

Most likely noone with more ram than 3 gb will ever have this issue, at all

Can we please stop speculating on specs and performance please? <3

@PhysSong
Copy link
Member

It seems fine to do nothing in Song::updateLength while loading projects and calling that once the loading finishes. Any opinions?

@michaelgregorius
Copy link
Contributor

@PhysSong I checked some more and found that the scrollbar of the Song Editor is not updated when I put a return at the top of Song::updateLength. This also matches the observation that the class SongEditor connects its updateScrollBar slot to the lengthChanged signal of the class Song. That signal is emitted at the end of Song::updateLength.

It also has to be made sure that all interactive cases where Song::updateLength is called still work. So far I have found the following methods which call Song::updateLength:

  • TrackContentObject::movePosition
  • TrackContentObject::changeLength
  • Track::removeTCO
  • ProjectRenderer::run

I assume that some of them are also called when the user makes some interactive changes to some track content objects.

@messmerd
Copy link
Member

Can this be closed now?

@Rossmaxx
Copy link
Contributor

I think so, yeah, but would defer this to @michaelgregorius as he did the work back then and seems a good fit.

@michaelgregorius
Copy link
Contributor

I think so, yeah, but would defer this to @michaelgregorius as he did the work back then and seems a good fit.

I in turn defer this question to @musikBear as they are the original reporter and can best state if their problem has been fixed or if there's still something that causing problems.

Scrolling through the comments it seems like lots of work has been done to improve performance.

@musikBear
Copy link
Author

Ancient and obsolete -Closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants