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
207 stars 9 forks source link

Reimplement the most important structs and traits for testing #19

Closed TimDiekmann closed 4 years ago

TimDiekmann commented 5 years ago

In order to test features, it makes sense to me to reimplement the important parts from rust-lang/rust. After changes to the API were decided in the discussions/issues, the code base should reflect those changes and everyone is aware of the current state.

Currently, this includes Alloc, AllocErr, CannotRealocInPlace, Excess, Layout, LayoutErr, Global and Box. Additionally, the following functions are adjusted to work with our Layout: alloc, alloc_zeroed, dealloc, realloc, and handle_alloc_error.

However, a few things had to be adjusted to match the alloc crate as soon as possible:

I did not set up an CI, this is up to the repository owner :slightly_smiling_face:

glandium commented 5 years ago

There's an independent, but stable-only, version of this in https://crates.io/crates/allocator_api

Ericson2314 commented 5 years ago

Yeah I think it's better to keep the code separate from the wg repo.

SimonSapin commented 5 years ago

@Ericson2314 Why?

TimDiekmann commented 5 years ago

There's an independent, but stable-only, version of this in https://crates.io/crates/allocator_api

@glandium I'm aware of your crate, but as you mentioned it's for the stable toolchain and liballoc is a crate which is using #![feature(...)] a lot. As we want to merge our changes to liballoc, we have to work around nightly features and later revert them. Additionally, I think you don't want to drop semver compatibility, and if only one of the filed issues will be used, it would break it.

Yeah I think it's better to keep the code separate from the wg repo.

@Ericson2314 I don't think we should split up code and discussions. With the code in this repo, it's possible to make PRs, discuss them, and merge the change if we want to keep those.

TimDiekmann commented 5 years ago

Another thing for discussion:

gnzlbg commented 5 years ago

If something has enough consensus to be merged here (and approval of the libs team), why wouldn't we want to perform the change in rust-lang/rust instead ?

TimDiekmann commented 5 years ago

If something has enough consensus to be merged here (and approval of the libs team), why wouldn't we want to perform the change in rust-lang/rust instead ?

Playing around with liballoc directly takes a lot of time to compile. Even if stage0 is not compiled, it's way faster to maintain a small crate with the basic functionality.

gnzlbg commented 5 years ago

So I don't know if this would be very useful. At best, this allows us to say: "hey, this change seems ok, for this thing that's not liballoc", which given how thorny allocators are, doesn't really need to translate to the real world in any meaningful way.

I would be on board with properly forking rust-lang/rust/src/liballoc for the purpose of experimentation somewhere, with its tests, benchmarks, and ideally, also implementations of the Alloc/GlobalAlloc traits for the most widely used allocators (heap, dlmalloc, jemalloc, wee_alloc, etc.), and their tests and benchmarks, as well as some integration tests (e.g., some servo benchmarks).

That would allow us to not only see how the changes proposed as PR impact not only some parts of liballoc, but all of liballoc, its users, run the tests, run the benchmarks, etc. to be able to have some hard data about their impact.

Whether we do this in this repo, or in a different one (e.g. rust-lang/wg-allocators-liballoc), I don't think matters much.

Ericson2314 commented 5 years ago

@SimonSapin so we can merge upstream into the code and make PRs from the code. All the things @gnzlbg said.

TimDiekmann commented 5 years ago

@SimonSapin so we can merge upstream into the code and make PRs from the code.

Why this is not possible with this repository? For now we only use this repository for the issue tracker.

Ericson2314 commented 5 years ago

If you merge this into master here as opposed to a separate branch, then when you go to merge back into the main repo it will pull in this readme and what-not?

gnzlbg commented 5 years ago

If we just fork rust-Lang/rust we could easily keep the repo in sync. For our purposes we don’t have to build stage0, only the liballoc crate via cargo using nightly rust, right?