bevy icon indicating copy to clipboard operation
bevy copied to clipboard

Optimize label comparisons and allow runtime formatting

Open joseph-gio opened this issue 3 years ago • 0 comments

Objective

  • Addresses concerns raised by cart in #5366.
  • Make label comparisons O(1).
  • More freedom for debug formatting.
  • Be less prescriptive -- why should everything have to be only a string?

Solution

  • Require implementers of Label to return a u64 value that distinguishes between labels of the same type.
  • For formatting, store an &mut Formatter fn pointer.
    • Smaller stack size than a string slice.
    • More freedom than storing a string slice -- debug strings can depend on runtime values or be composed generically.
  • The concept of labels not being allowed to hold data now applies only for the derive macro. Custom implementations can store whatever they want as long as they can fit it in a u64.
// Manual implementation
enum MyLabel {
    One,
    Two,
}
impl SystemLabel for Mylabel {
    fn data(&self) -> u64 {
        match self {
            Self::One => 0,
            Self::Two => 1,
        }
    }
    fn fmt(data: u64, f: &mut fmt::Formatter) -> fmt::Result {
        match data {
            0 => write!(f, "MyLabel::One"),
            1 => write!(f, "MyLabel::Two"),
            _ => unreachable!(),
        }
    }
}

Notes

  • Blocked by #4341
  • Label traits are no longer object safe, but this is probably a good thing since it makes it easier to catch obsolete instances of Box<dyn Label>. LabelId should always be used instead.

Editor support

We can easily support labels determined at runtime by storing a bunch of Strings in a RwLock and storing the index in the label. Then, Label::fmt would just acquire the lock and look up the associated string. This is potentially slow, but label names are only used for debugging.

Benchmarks

# of Systems No deps % Change W/ deps % Change
100 199.20 us -4.3151% 1.7361 ms -5.3507%
500 765.43 us -4.2735% 59.937 ms -4.8282%
1000 1.4137 ms -3.8197% 299.22 ms -4.1718%

Changelog

  • Removed as_str() from label traits.
  • Added data() and fmt() to label traits.
  • Format generics in Label derive macros.

Migration Guide

The Label family of traits (SystemLabel, StageLabel, etc) is no longer object safe. Any use of Box<dyn Label> should be replaced with LabelId - this type serves the same purpose but does not require allocations. Any use of &dyn Label should be replaced with impl Label if possible, or LabelId if you cannot use generics.

For manual implementers of Label traits you must now implement the data() method, which returns an integer used to distinguish between labels of the type same type.

Additionally, you must now implement the fmt() associated fn instead of as_str(). This method prints to an &mut Formatter, and is passed the result of calling .data() to enable runtime formatting.

joseph-gio avatar Jul 19 '22 09:07 joseph-gio