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

reduce temporary allocations in Color serde impl #925

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

Conversation

kristopherbullinger
Copy link

hello. i have made some small changes in the impl of Serialize and Deserialize for crossterm::style::Color which reduces the number of temporary allocations created. To achieve this, I have:

  • replaced calls to str::replace with other suitable methods such as str::strip_{suffix|prefix} in the deserialize impl
  • replaced a collected-into vec with manually polling an iterator 4 times and applying pattern matching in the deserialize impl
  • used a stack-based string for formatting ansi and rgb strings instead of the string-based format! macro

For ease of implementation, I have brought in the arrayvec dependency, dependent on the serde crossterm feature. This enables using the write! macro on an arrayvec::ArrayString rather than fiddling with an array on the stack and having to uphold utf8 invariants manually. The length of the stack-based string is based on the longest possible formatted color, which is "rgb_(255,255,255)", at length 17.

The deserialize tests as written are all still passing, and a manual check of the serialize impls confirms that colors Color::Rgb{ r: 255, g: 255, b: 255 }, Color::Ansi(1), and Color::Red still serialize as expected (debug printing shown):

Ok("\"rgb_(255,255,255)\"")
Ok("\"ansi_(1)\"")
Ok("\"red\"")

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.

1 participant