Skip to content

Conversation

@CatThingy
Copy link
Contributor

@CatThingy CatThingy commented Mar 7, 2023

Objective

Solution

  • Add an associated type on Event for the storage type to use

Changelog

  • Added the ability to let events be stored in a SmallVec

@james7132 james7132 added C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events labels Mar 7, 2023
@alice-i-cecile alice-i-cecile added the X-Controversial There is active debate or serious implications around merging this PR label Mar 7, 2023
@alice-i-cecile alice-i-cecile added this to the 0.11 milestone Mar 7, 2023
@james7132
Copy link
Member

Thinking a bit harder on this, I want us to have quantifiable metrics via #7731 for this. The extra branch on the configuration (if we can't eliminate it) is a constant cost regardless of whether we use smallvec or not. If the memory saving benefits are more than worth the CPU cost, I'd say we should still merge.

@james7132 james7132 added the S-Needs-Benchmarking This set of changes needs performance benchmarking to double-check that they help label Mar 9, 2023
@alice-i-cecile alice-i-cecile removed this from the 0.11 milestone Jun 19, 2023
@CatThingy CatThingy marked this pull request as draft September 13, 2023 01:28
@BenjaminBrienen BenjaminBrienen added D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Jan 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ECS Entities, components, systems, and events C-Feature A new feature, making something new possible D-Modest A "normal" level of difficulty; suitable for simple features or challenging fixes S-Needs-Benchmarking This set of changes needs performance benchmarking to double-check that they help S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged X-Controversial There is active debate or serious implications around merging this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow opt-in configuration of events to use SmallVec

4 participants