Skip to content

ipnetwork: Deserialize str instead of allocating#45

Open
joshuamegnauth54 wants to merge 1 commit into
rust-stdx:mainfrom
joshuamegnauth54:ipnetwork-serde-borrow
Open

ipnetwork: Deserialize str instead of allocating#45
joshuamegnauth54 wants to merge 1 commit into
rust-stdx:mainfrom
joshuamegnauth54:ipnetwork-serde-borrow

Conversation

@joshuamegnauth54

Copy link
Copy Markdown

Currently, ipnetwork always allocates during deserialization. This is unfortunate because ownership isn't needed to construct any of ipnetwork's types.

I implemented a small Visitor that opportunistically deserializes from a str borrowed from the deserializer. For some formats, this should help zero alloc deserialization. While some formats will still allocate and pass the deserializer a &str, this Visitor still avoids the second alloc from String in the original.

@sylvain-pingoo

Copy link
Copy Markdown
Member

Hello, and thank you for the contribution.

I may be mistaken but it looks like that you didn't use make fmt to format the code and probably used cargo fmt instead, which is adding some noise to the PR (and please don't forget to merge / any additional commit into a single commit). Thank you!

Currently, `ipnetwork` always allocates during deserialization. This is
unfortunate because ownership isn't needed to construct any of
`ipnetwork`'s types.

I implemented a small Visitor that opportunistically deserializes from a
str borrowed from the deserializer. For some formats, this should help
zero alloc deserialization. While some formats will still allocate and
pass the deserializer a &str, this Visitor still avoids the second alloc
from String in the original.
@joshuamegnauth54 joshuamegnauth54 force-pushed the ipnetwork-serde-borrow branch from 46ca21a to e7f086c Compare June 23, 2026 01:32
@joshuamegnauth54

Copy link
Copy Markdown
Author

Hi! Sorry, I totally missed the notification for your comment. You're right that I formatted with stable rustfmt originally. I didn't notice make fmt was different.

Anyway, I formatted with nightly and force pushed.

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