rust-lang / wg-allocators

Home of the Allocators working group: Paving a path for a standard set of allocator traits to be used in collections!
http://bit.ly/hello-wg-allocators
209 stars 9 forks source link

Semantics on allocating and deallocating with different `Allocator` #133

Open Ddystopia opened 2 weeks ago

Ddystopia commented 2 weeks ago

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:

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 2 weeks ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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 commented 1 week ago

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.