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

Enable sorting for walk_ports #60

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Enable sorting for walk_ports #60

wants to merge 1 commit into from

Conversation

JohannesLorenz
Copy link
Collaborator

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

@fundamental
Copy link
Owner

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
Copy link
Collaborator Author

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
Copy link
Owner

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

@JohannesLorenz
Copy link
Collaborator Author

@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
Copy link
Owner

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
Copy link
Collaborator Author

Blocked by zynaddsubfx/zynaddsubfx#197 .

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

You probably meant harmless stack allocations, right?

@fundamental
Copy link
Owner

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.

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

Successfully merging this pull request may close these issues.

2 participants