rtosc icon indicating copy to clipboard operation
rtosc copied to clipboard

Enable sorting for walk_ports

Open JohannesLorenz opened this issue 3 years ago • 7 comments

Tested with zyn: The sorted schemata before and after the PR are equal (except of the proposed whitespace fix).

JohannesLorenz avatar Jan 23 '22 10:01 JohannesLorenz

As it stands this can't be merged. The additional complexity introduces allocations into port walking which is used in a very small number of spots for RT port reflection (I think in at least one of the MIDI mapper/automations code implementations). Seeing the relative complexity here I think it might make sense to implement the sort at the level of the schema dumping on zyn's side.

fundamental avatar Jan 29 '22 16:01 fundamental

The additional complexity introduces allocations into port walking which is used in a very small number of spots for RT port reflection (I think in at least one of the MIDI mapper/automations code implementations).

They should all have bool sorted = false, which means there will be no vector allocated. Or which allocation do you mean?

JohannesLorenz avatar Jan 29 '22 16:01 JohannesLorenz

True, but you know about how many headaches that creates with stoat.

fundamental avatar Jan 29 '22 17:01 fundamental

@fundamental What about using a stack allocation for that vector? We know the size already, and stack allocations are common in rtosc. Is the heap allocation of std::vector<const Port*> subports_sorted your only concern?

JohannesLorenz avatar Apr 16 '22 10:04 JohannesLorenz

Allocations are the primary concern with added complexity being a smaller secondary concern. stack allocation is a possibility for subports_sorted, though std::stable_sort is not guaranteed to be alloc free, and C++ lambdas will generate allocations if there are captured variables.

fundamental avatar Apr 16 '22 22:04 fundamental

Blocked by zynaddsubfx/zynaddsubfx#197 .

C++ lambdas will generate allocations if there are captured variables

You probably meant harmless stack allocations, right?

JohannesLorenz avatar May 01 '22 19:05 JohannesLorenz

You probably meant harmless stack allocations, right?

Last I checked (which to be fair was a good few years back) it was heap allocations. The design choice was made at some level to simplify how lambdas are routinely packed into std::function<> objects. My info could be out of date though.

fundamental avatar May 04 '22 21:05 fundamental