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

feature: Create a new heap variant to replace Arc<str> #30

Merged
merged 11 commits into from
Dec 9, 2021
Merged

Conversation

ParkMyCar
Copy link
Owner

@ParkMyCar ParkMyCar commented Dec 8, 2021

This PR changes our HeapString implementation from using an Arc<str> to a custom defined ArcString type. This type is more analogous to Arc<[u8]>, specifically it allocates a buffer which may be larger than the "string" it contains. This gets us closer to being a true drop-in replacement for String and allows us to write APIs for mutating a CompactStr

@ParkMyCar ParkMyCar changed the title Create a new heap variant to replace Arc<str> feature: Create a new heap variant to replace Arc<str> Dec 8, 2021
@NobodyXu
Copy link
Contributor

NobodyXu commented Dec 9, 2021

I think it’s totally unnecessary to reinvent Arc here.

We can use slice-dst to create an unsized Arc.

Or, we can use triomphe which provides similar functionality but easier to use, and I had my PR relating to MaybeUninit usage merged in it.

@ParkMyCar
Copy link
Owner Author

Thanks for the feedback @NobodyXu!

I did look at tripomphe and I actually saw your recent PR for adding an API for MaybeUninit<T>, but unfortunately it doesn't seem to have been released yet? Otherwise I played around with trimophe::ThinArc a bit but it doesn't provide an API to modify the underlying buffer, which is one of the goals of this change, so we can get closer to being a truly drop-in replacement for String.

@ParkMyCar ParkMyCar merged commit 41bc283 into main Dec 9, 2021
@NobodyXu
Copy link
Contributor

an API to modify the underlying buffer

Is that for the copy-on-write usage?

Well then, I think this does require creation of another unique Arc type, though I think it might be better to implement this in another crate in a generic way.

But then, that can be postponed until this feature is completed in compact_str.

This is quite a novel way to implement CompactStr to save memory while permit modification.

Thanks for taking your time to answer my question!

@ParkMyCar ParkMyCar deleted the heap_str branch December 23, 2021 20:07
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.

None yet

2 participants