-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Add generic parameter for an allocator to collection types and box-like structures. #71873
Conversation
@bors try |
⌛ Trying commit 08a2371d676626cb15c041414bc5c092d4c70296 with merge a6b9054912d7518df0cd0c0050d5c5b1a51b9b14... |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
☀️ Try build successful - checks-azure |
How the Try build can succeed, but the PR build fails? (locally, |
Rebased onto master and fixed 32bit output (Try-build only checks 64bit) |
@craterbot check |
👌 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
Actually, I think we should go ahead and add allocator parameters to |
Historically we've also held off on doing so because it was a pretty sizeable regression on perf, IIRC -- maybe we should check; @rust-timer queue a6b9054912d7518df0cd0c0050d5c5b1a51b9b14 |
Awaiting bors try build completion |
er I meant @rust-timer build a6b9054 |
Queued a6b9054912d7518df0cd0c0050d5c5b1a51b9b14 with parent d6823ba, future comparison URL. |
One crater run seems good, but for performance measurement I don't think it's the best option. As the timer is queued, I'll add a parameter to the other types. Also, does hashbrown currently support any type of allocator? I'd leave that out for now. |
You don't need to actually use the allocator parameter. We're just checking if adding the parameter with a default value will break existing code. |
I noticed that most of the collections uses methods like |
|
I added a parameter to |
Box
Finished benchmarking try commit a6b9054912d7518df0cd0c0050d5c5b1a51b9b14, comparison URL. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Rebased, but I think I need some help with the chalk-error. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
☔ The latest upstream changes (presumably #72013) made this pull request unmergeable. Please resolve the merge conflicts. |
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
🚨 Experiment 🆘 Can someone from the infra team check in on this? @rust-lang/infra |
@craterbot start=master#7c59a81a5fcbaaca311f744cd7c68d99bfbb05d3 end=try#d93021f5faa26e4f9d2719e79f9974e06aa40e17 The branch name still needs to be added. |
🚨 Error: it's only possible to edit queued experiments 🆘 If you have any trouble with Crater please ping |
@craterbot start=master#7c59a81a5fcbaaca311f744cd7c68d99bfbb05d3 end=try#d93021f5faa26e4f9d2719e79f9974e06aa40e17 |
📝 Configuration of the ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
🚧 Experiment ℹ️ Crater is a tool to run experiments across parts of the Rust ecosystem. Learn more |
@nikomatsakis ping about the chalk error
|
🎉 Experiment
|
The "unknown" are a Crater bug, we had so many Crater bugs with this run :( |
The chalk error is legit in the sense that we haven't defined what fundamental means for a type with multiple type parameters. I guess we will ignore all but the first, but we're going to have to spend at least some time thinking about how to use this to break coherence :) |
I finally had time to get through the crater report. There are 100 regressed crates, and almost everyone failed because However, one crate failed because of implementing As this is still blocked on #72314 and the chalk error, I'll close this. Actually, this PR was never designed to be merged anyway 😉 |
Support custom allocators in `Box` r? `@Amanieu` This pull request requires a crater run. ### Prior work: - rust-lang#71873 - rust-lang#58457 - [`alloc-wg`](https://github.com/TimDiekmann/alloc-wg)-crate Currently blocked on: - ~rust-lang#77118~ - ~rust-lang/chalk#615 (rust-lang#77515)~
Add support for custom allocators in `Vec` This follows the [roadmap](rust-lang/wg-allocators#7) of the allocator WG to add custom allocators to collections. r? `@Amanieu` This pull request requires a crater run. ### Prior work: - rust-lang#71873: Crater-test to solve rust-lang/wg-allocators#1 - [`alloc-wg`](https://github.com/TimDiekmann/alloc-wg)-crate
This adds a generic parameter
A: AllocRef
toBox
. This pull request mainly was made to run crater to test compatibility when adding a defaulted generic parameter to one of the most used structs. It's not yet possible to construct aBox
with a generic allocator.cc rust-lang/wg-allocators#1
r? @Amanieu