Skip to content

remove unnecessary allocations in float and int serialization via SerJson and SerRon - #151

Merged
knickish merged 1 commit into
not-fl3:masterfrom
tadeohepperle:master
Apr 12, 2026
Merged

remove unnecessary allocations in float and int serialization via SerJson and SerRon#151
knickish merged 1 commit into
not-fl3:masterfrom
tadeohepperle:master

Conversation

@tadeohepperle

@tadeohepperle tadeohepperle commented Feb 15, 2026

Copy link
Copy Markdown
Contributor

It is quite wasteful to allocate a new string for every integer or float encountered during serialization to json (or Ron) via e.g. format!("{self:?}").

I replaced all relevant instances by write!(s.out, "{self:?}"), which formats into the output string directly without any transient allocation. If there were any benchmarks I am pretty sure we'd see performance improvements.

Comment thread src/toml.rs
'\u{41}'..='\u{5A}'
| '\u{61}'..='\u{7A}'
| '\u{30}'..='\u{39}'
| '\u{2D}'

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whats up with this one? Is this removal intentional?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, clippy told me that the pattern is unreachable because it is covered in an earlier branch, so I just removed it:

warning: unreachable pattern
   --> src/toml.rs:15:11
    |
 15 |         | '\u{2D}'
    |           ^^^^^^^^ no value can reach this
...
590 |             '-' => {
    |             --- matches all the relevant values
...
601 |             ident_chars!() => return self.parse_ident(i, num),
    |             -------------- in this macro invocation

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@not-fl3 Would it be possible for you to merge this MR already?

@knickish
knickish merged commit 89c1028 into not-fl3:master Apr 12, 2026
16 checks passed
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.

3 participants