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

Custom allocation for integers #43

Open
mthom opened this issue Nov 29, 2023 · 7 comments
Open

Custom allocation for integers #43

mthom opened this issue Nov 29, 2023 · 7 comments

Comments

@mthom
Copy link

mthom commented Nov 29, 2023

In scryer-prolog I want to allocate integers to a custom garbage-collected arena so that the memory of unreachable (but always immutable) integers and rationals can be implicitly recycled without calling any Drop instance. dashu doesn't currently have a means to customize allocation.

I'd like to submit a PR to add the feature to dashu, likely by elevating MemoryAllocation to a trait whose instance can be specified in the ibig! and ubig! macros among other places, at least until the allocator_api is merged to stable (probably not for a long while yet).

Is this acceptable @cmpute ? Or perhaps there's a way to go about this that I don't know about?

@cmpute
Copy link
Owner

cmpute commented Nov 30, 2023

Hi thanks for your idea! I'm not sure.. If you want a custom allocator, that means you have to add a type parameter to the integer type right? In that case I'm afraid that it will make a huge break change and make the library much less friendly to users.

Could you please provide a small snippet of how you would like to implement this feature? (e.g. how you will instrument the structs). With that context, I can have a better idea of how large this change will be. Thanks again!

@mthom
Copy link
Author

mthom commented Nov 30, 2023

My solution would introduce derived integer and rational types that would indeed be parameterized on an allocator type. The derived types would be composed of e.g. an IBig value and an allocator value. I only need the immutable interface on IBig (methods of &self) but I'd add mutable methods for the sake of completeness.

I'm not sure to what degree this would require duplicating existing code. It appears much of it is generated by macros already. Maybe if it was behind an optional feature turned off by default?

@cmpute
Copy link
Owner

cmpute commented Dec 1, 2023

It will be great if additional types (such as IBigWithAlloc or sth) can be introduced without break changes to the current code, that will be great. However, I'm also somewhat concerned about the amount of work needed. If you want to take a look into this, maybe we can have a draft PR with minimal implementations, and then we can evaluate the possibilities.

Regarding using a feature flag, it's not ideal since feature flags have to be additive (i.e. turning on the flag should not make existing code break)

@mthom
Copy link
Author

mthom commented Dec 5, 2023

Ok, I'll submit a draft PR somewhat soon (in the coming weeks or months).

@mthom
Copy link
Author

mthom commented Dec 5, 2023

IBig and UBig, etc. could be parameterized with an additional allocator type but would be exported from the library as they are (i.e. without type parameters, as unparameterized type aliases) using a zero-sized allocator type that implements the default behavior. Nothing in the interface would change, it would break nothing, but the interface would be extended to support construction with a custom allocator object.

Would that acceptable? I think that would be a lot less impactful than introducing an immutable integer type with customizable allocation.

@cmpute
Copy link
Owner

cmpute commented Dec 6, 2023

Your plan sounds good to me. I imagine that signatures of the constructors might change, but it makes a minimal impact to the users of the dashu meta package, and it's acceptable since dashu is still pre-1.0.

Regarding the allocator type, what's your opinion on the trait definition? Do you plan to use the std::alloc::Allocator or a custom trait? IMO, we can use a custom trait definition (maybe located in the dashu-base crate, that mimic the interface of std::alloc::Allocator , so that when std::alloc::Allocator is stablized, we can adapt to use that trait with minimal efforts.

@mthom
Copy link
Author

mthom commented Dec 6, 2023

Regarding the allocator type, what's your opinion on the trait definition? Do you plan to use the std::alloc::Allocator or a custom trait? IMO, we can use a custom trait definition (maybe located in the dashu-base crate, that mimic the interface of std::alloc::Allocator , so that when std::alloc::Allocator is stablized, we can adapt to use that trait with minimal efforts.

Yes, exactly this.

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

No branches or pull requests

2 participants