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

Semantics on allocating and deallocating with different Allocator #133

Open
Ddystopia opened this issue Sep 1, 2024 · 11 comments
Open

Semantics on allocating and deallocating with different Allocator #133

Ddystopia opened this issue Sep 1, 2024 · 11 comments

Comments

@Ddystopia
Copy link

Ddystopia commented Sep 1, 2024

I propose to explicitly allow to allocate and deallocate with different implementations of Allocator trait under specified circumstances. I'm not sure how exactly it must be done, but I guess it would not increase complexity of Allocator trait while provide some benefits:

  • It can help Vec too, because on resize Vec is passing old pointer, so allocator might not need to store pointers to original data storage, reducing size of Vec<T, A>.
  • Resolve Split Allocator trait #112, because bumpalo can just allocate in Box<T, Bump> and then convert to Box<T, Noop>.
  • Kind of allow &move T, because it's semantics are quite similar to Box<T, Noop>.
  • Increase flexibility.

Maybe conversion can be allowed only via impl From<Box<T, A1>> for Box<T, A2> where A1: From<A2>, or just allow (declare sound) to Box::from_raw_in(Box::into_raw(box)).

@Lokathor
Copy link

Lokathor commented Sep 1, 2024

I'm not sure how exactly it must be done, but I guess it would not increase complexity of Allocator trait while provide some benefits

It's extremely dubious to say that you don't know how, but you also don't think it'll increase complexity.

I would go so far as to say it fundamentally doesn't make sense to deallocate with a different allocator than you allocated with. The two allocators don't know about each other, so the memory still can't get back where it's supposed to.

@Ddystopia
Copy link
Author

I would go so far as to say it fundamentally doesn't make sense to deallocate with a different allocator than you allocated with. The two allocators don't know about each other, so the memory still can't get back where it's supposed to.

I'm not sure what is the problem in this? Of course just allowing deallocate in any allocator is not an option, but but declaring it sound under controlled circumstances should not raise a fundamental question. The core problem is that you can't change a layout during allocator's lifecycle. There are even use cases like arenas present, or my use case with single use allocators with space in statics, and potentially more as people start exploring possibilities allocator api gives them.

It's extremely dubious to say that you don't know how, but you also don't think it'll increase complexity.

I agree, I personally don't want to push in any direction about implementation details, just discuss the matter.

@Lokathor
Copy link

Lokathor commented Sep 1, 2024

Two specific allocator implementations are free to try and be compatible with each other, but that would definitely not be part of the general Allocator trait that all general allocators implement.

@Ddystopia
Copy link
Author

Ddystopia commented Sep 2, 2024

Two specific allocator implementations are free to try and be compatible with each other, but that would definitely not be part of the general Allocator trait that all general allocators implement.

Seems good to me. What I'm trying to propose is to not forbid this kind of behavior and not declare it undefined behavior, and explicitly documenting this possibility.

Today it is undefined behavior, as deallocate has this like in documentation:

ptr must denote a block of memory currently allocated via this allocator

As well as grow, grow_zeroed, shrink.

Changing "this allocator" to something that will include "Two specific allocator implementations are free to try and be compatible with each other" should be enough.

@Lokathor
Copy link

Lokathor commented Sep 2, 2024

If two allocators wanted to coordinate they would have to do it via some way that is not the Allocator trait.

The Allocator trait is about a general API, providing essentially the "minimum standard" that callers can rely on.

@Ddystopia
Copy link
Author

If two allocators wanted to coordinate they would have to do it via some way that is not the Allocator trait.

The Allocator trait is about a general API, providing essentially the "minimum standard" that callers can rely on.

Can the following code be allowed then?

// context: foo, A1, A2 are part of a single crate
fn foo<T>(val: T, a1: A1, a2: A2) -> Box<T, A2> {
    let first = Box::new_in(val, a1);
    let second = Box::from_raw_in(Box::into_raw(first), a2);
    second
}

@CAD97
Copy link

CAD97 commented Sep 2, 2024

Can the following code be allowed then?

Yes. Concrete implementations may always choose to provide stronger guarantees about their functionality than is required by the trait's general guarantees. Of course, this should be documented, and documentation attached to a trait impl is, while available, not super visible, but this is completely standard practice.

In fact, you likely rely on this exact kind of refinement today. By the docs of Allocator alone, it isn't valid to do Box::from_raw(Box::into_raw(b)), because the new box uses a different instance of Global as the allocator. But it is allowed, because Box, Global, and the free functions in std::alloc all provide a guarantee that they are compatible with each other.

When writing a generic function, you program against the generic interface. When using a known concrete type, you program against the concrete implementation. (Although you must keep in mind what guarantees it actually gives in the face of future development evolution.)

@Ddystopia
Copy link
Author

Ddystopia commented Sep 2, 2024

Can the following code be allowed then?

Yes. Concrete implementations may always choose to provide stronger guarantees about their functionality than is required by the trait's general guarantees. Of course, this should be documented, and documentation attached to a trait impl is, while available, not super visible, but this is completely standard practice.

In fact, you likely rely on this exact kind of refinement today. By the docs of Allocator alone, it isn't valid to do Box::from_raw(Box::into_raw(b)), because the new box uses a different instance of Global as the allocator. But it is allowed, because Box, Global, and the free functions in std::alloc all provide a guarantee that they are compatible with each other.

When writing a generic function, you program against the generic interface. When using a known concrete type, you program against the concrete implementation. (Although you must keep in mind what guarantees it actually gives in the face of future development evolution.)

Thank you for your answer!

To clarify, the issue is about "magic" that might be involved in allocators. For example, Global is magic. Can user be sure that violating lang trait's safety documentation not leading to immediate ub?

@zakarumych
Copy link

Regarding Split Allocator trait #112, the problem is Noop being Allocator, as it would need to implement allocate method and do what, panic?
So <Box<T, Noop> as Clone>::clone - quite innocent looking call would panic? The whole idea behind Deallocator trait is to prevent one from calling function that needs to allocate and allow only to deallocate.

For the purpose of compatibleness, Allocator cloned from one that was used to allocate is considered compatible, and I propsed that allocator produced by From/Into is also compatible to deallocate.

But I don't see how exactly changing type of Allocator to another Allocator can benefit anything. For Deallocator it is obvious that some deallocators might need less state to only deallocate.

@Ddystopia
Copy link
Author

Regarding Split Allocator trait #112, the problem is Noop being Allocator, as it would need to implement allocate method and do what, panic? So <Box<T, Noop> as Clone>::clone - quite innocent looking call would panic? The whole idea behind Deallocator trait is to prevent one from calling function that needs to allocate and allow only to deallocate.

For the purpose of compatibleness, Allocator cloned from one that was used to allocate is considered compatible, and I propsed that allocator produced by From/Into is also compatible to deallocate.

But I don't see how exactly changing type of Allocator to another Allocator can benefit anything. For Deallocator it is obvious that some deallocators might need less state to only deallocate.

I agree that splitting those traits would be more beneficial, but it seems unlikely to me that it would get accepted, at least if there will be no more motivating use cases for such a big change and increase of complexity. If we are speaking about performance, it would not make a difference: have a dedicated Deallocator or Noop implementation of Allocator.

@zakarumych
Copy link

If we are speaking about performance, it would not make a difference: have a dedicated Deallocator or Noop implementation of Allocator.

Performance yes, but it would be a footgun. So it's better to use not a Box for arena allocators that has noop deallocation.

In my code I just went different route and implemented allocator that returns &mut T and drops value on reset.

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

4 participants