fundamental / rtosc

Realtime Safe OSC packet serialization and dispatch
https://fundamental-code.com/wiki/rtosc/
MIT License
85 stars 16 forks source link

Enable sorting for walk_ports #60

Open JohannesLorenz opened 2 years ago

JohannesLorenz commented 2 years ago

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

fundamental commented 2 years ago

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.

JohannesLorenz commented 2 years ago

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?

fundamental commented 2 years ago

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

JohannesLorenz commented 2 years ago

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

fundamental commented 2 years ago

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.

JohannesLorenz commented 2 years ago

Blocked by zynaddsubfx/zynaddsubfx#197 .

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

You probably meant harmless stack allocations, right?

fundamental commented 2 years ago

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.