samplers do not scale well

[Be]

Deere and Tango now can show 64 samplers. In regards to the engine, 64 samplers is no big deal if samplers can't be assigned to effects, but loading the skin widgets for 64 samplers is a major bottleneck for Mixxx's startup and shutdown times. If you comment out the whole sampler template in Deere or Tango they load and shut down *much* faster. Using completely separate templates for collapsed and expanded sampler views exacerbates this. Let's decide on which sampler features are strictly necessary to show in skins. We can keep the collapsed/expanded view toggling, but the expanded view should show a secondary WidgetGroup below what is shown in the collapsed view, not replace the whole sampler with a different WidgetGroup.

[Daniel Schürmann (daschuer)]

Aren't collapsed regions parsed anyway?
I am afraid we can only slice this by introducing a michanism that allows to skip skin regions by a preferences option or something. An other idea is to allow to duplicate an ready initialised widget in memory. Mmm .. all a lot of work.

[Be (be.ing)]

> Aren't collapsed regions parsed anyway?

Yes.

> An other idea is to allow to duplicate an ready initialised widget in memory. Mmm .. all a lot of work.

The sampler rows are already in singleton widgets.

I think if we remove widgets from the samplers this could work. Every widget that is added to the sampler template is instantiated 64 times. Even if we reduce the number of samplers to 32 or 16 or 8 we still have issues with scaling.

[ronso0 (ronso0)]

What do you mean by *much faster*?

With 64 samplers mixxx is ready in 12sec,
with just 8 samplers it takes 10sec.

- I set "[Master],num_samplers" to 8
- commented SingletonDefinitions for sampler row 2-8
- commented sampler rows 2-8

[Be (be.ing)]

> What do you mean by *much faster*?

Running mixxx with --logLevel debug and Tango, it takes about 10 seconds to delete the skin (measured by the time reported with "deleting menubar"). Removing everything but the <Template> tag from sample_decks.xml, that goes down to 3.6 seconds.

[Be (be.ing)]

We have a scaling issue with just 8 samplers, so I don't think reducing the number of samplers is a good solution.

[ronso0 (ronso0)]

You're right, shutting down & reloading a skin is much faster.

[ronso0 (ronso0)]

I don't really see which Widgets could be removed from Tango samplers without cutting expected features.
The only element I'm not sure about are the hotcue buttons, though I doubt that significantly improves loading time (didn't find a good way to accurately measure the loading time):
As a workaround for long tracks with where the samplers cue point should be altered in samplers, those tracks could be loaded into decks first, be prepared and the dragged to any sampler deck.

[ronso0 (ronso0)]

A global approach could be to set the number of samplers in the Preferences window (8 being the default?) while skins could define the maximum. Controller mappings could overwrite this value if a controller is built to support more samplers.
I guess this would accelerate loading on slower machines (8 vs. 64 samplers in Tango and Deere)

[ronso0 (ronso0)]

I wanted to check if a sampler rework is really worth the effort, so I tested this with my stopwatch.
Via commandline I started just 'mixxx' with all 64 samplers loaded, all have cover art available.

'mini + maxi' means normal samplers with separate templates for collapsed and expanded version.

             start shutdown
mini + maxi 13s 28s
mini only 12s 13s
maxi only 12s 23s
no samplers 12s 10s

As you can see only shutdown time is affected, startup time stays almost steady.
What's done when a skin is deleted? Why does it take so much longer than building it?
Maybe it's better to optimize that process instead of pruning the samplers and cutting usability.

[ronso0 (ronso0)]

test was done with Tango btw

[Be (be.ing)]

My first thought was the saving the loaded samples to the samplers.xml file could be slowing it down, but I think that would happen even if the sampler skin widgets weren't loaded. Have you tested commenting out the whole sampler section of the skin?

[Be (be.ing)]

I did some further testing to determine the scaling issues with the skin widgets versus instantiating so many samplers in the engine. I commented out the setting of [Master],num_samplers in Deere's skin manifest and the sample_decks.xml template, plus deleted my samplers.xml file in my configuration folder. With this setup, Mixxx took about 325 MB resident memory and deleting the skin on shutdown took about 3 seconds. Uncommenting the sample_decks.xml template from the skin kept memory usage the same but increased shutdown time by a whole second. Uncommenting the setting of [Master],num_samplers to 64 in the skin manifest brought memory usage up to 575 MB and shutdown time was the same.

[Peter Burg (wunjo)]

Hi, in mixxx-2.1.0-alpha-pre-master-git6402-release-x64.exe Deere skin, volume and hotcue buttons are lost.

In mixxx-2.1.0-alpha-pre-master-git6395-release-x64.exe they were still there.
Inbetween versions not checked.
Thanks for the good work!
Win7-64 AMD 4 core 3300 8gb

[ronso0 (ronso0)]

For now I vote for limiting the number of samplers.
On my machine (quadcore @1.8GHz) it now takes around 30 seconds to shutdown when I enable all 64 samplers in Tango...

IMO only a minority of users is expecting a total of 64 samplers, while all the others have to deal with the resulting longer start/shutdown times. As I mentioned, I don't expect cutting sampler features is good approach either, see complaint above.

I see that we can't create a mechanism to set the numbers of samplers dynamically via Preferences > Interface or in another way with reasonable effort, and neither will we come come up with a good solution to the underlying skin problem soon.

But we can limit the number of samplers to 16 for Tango and Deere, and create duplicate skins called Deere (64 samplers) and Tango (64 samplers). The maintanance load would be minimal.
This way it would absolutely clear for the users, and the majority would benefit until we come up with a general solution.

[Daniel Schürmann (daschuer)]

Duplicating the skins will work for me. We but we should only duplicate the a single skin file.
Thinking about it, redirecting to a different skin folder is probably the same work than a preferences option .... what do you think.

[Be (be.ing)]

I have thought about the duplicate skins solution. It is ugly, but it may be the best option for now...

[Peter Burg (wunjo)]

From my perspective I can't imagine using more than 16 samplers and I definitely use the volume knob. Reducing the memory footprint is surely a good thing so duplicating skins might be fine :-)

[ronso0 (ronso0)]

Cool, feels like we have a compromise!

All skins are less than 1MB. What are your concerns to copy the whole skin folder and just alter the skin name in skin.xml?

In Deere and Tango we need to change two files: sample_decks.xml and corresponding skin menu button that sets the numbers of sampler rows. I'd use sample_decks.xml as switchpoint that loads either sample_decks_16.xml or ~_64.xml and do the same with the row selection button.

I already doing this for my personal branch. I can implement the changes for both Deere and Tango before the friday ;)

[Be (be.ing)]

My concern with duplicating skins is that every little change would have to be made to both copies of the skin, which would be a big hassle.

"I'd use sample_decks.xml as switchpoint that loads either sample_decks_16.xml or ~_64.xml and do the same with the row selection button."

Hmm, that gives me an idea. We could make a new preference option to select the number of samplers and have the skin load a different template depending on the preference option.

[Be (be.ing)]

This issue is annoying, but IMO it is not such a big deal that it needs to be rushed for the beta release. We can take care of it during the beta period.

[ronso0 (ronso0)]

> My concern with duplicating skins is that every little change would have to be made to both copies of the skin, which would be a big hassle.

No hassle at all: after a skin was changed (64 sample decks), just clone it to replace the old version with 16 samplers, then
* skin.xml > "[Master],num_samplers": change 64 to 16
* skin.xml > "sample_decks_XX.xml": change 64 to 16
* skin_settings_sampler_rows.xml: change 64 to 16

I'll put a patch into each skin directory.
I'm knee-deep in the Mixxx skins right now, so it'll take less time than writing this.

[ronso0 (ronso0)]

> Hmm, that gives me an idea. We could make a new preference option to select the number of samplers and have the skin load a different template depending on the preference option.

Nice, that's pretty much what I suggested here a month ago ;)
Also, the button (button stack) to set the number of sampler rows can be instantiated as Singleton in skin.xml, so it would be really just one file to change.

[ronso0 (ronso0)]

It's working fine.
Though, this is only a workaround, no matter if it's an option in preferences or a duplicate skin, so I won't assign this bug to me.

In the next days I'll open PRs for Deere and Tango with this change and small fixes, so both (and LateNight) should be in good shape when beta is released. Afterwards I'll have to design in the so-called real world again and won't have much time for skin design..

[Daniel Schürmann (daschuer)]

I am pro a persistent preferences option in the view menu. This is are just a view lines and can be done before beta. Defaulting to 8 sounds reasonable. What do you think?

[Be (be.ing)]

Adding a combobox to the preferences would be easy. But how would skins make use of it to load a different template depending on the preference?

[Be (be.ing)]

It might be a good idea to restrict the number of samplers in the engine in PlayerManager::addSampler to respect the preference option. That would limit RAM wasted by unused samplers.

[Daniel Schürmann (daschuer)]

I think we need a conditional widgets. That are only instance instead of a connected co has a certain value.

[Be (be.ing)]

Lazy loading singletons: https://github.com/mixxxdj/mixxx/pull/1426

[RJ Skerry-Ryan (rryan)]

Due to lack of progress, marking Triaged and clearing assignee. Feel
free to revert if it is in fact still in progress :).

[Swiftb0y (swiftb0y)]

Mixxx now uses GitHub for bug tracking. This bug has been migrated to:
https://github.com/mixxxdj/mixxx/issues/8919