-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Do NOT Review: CoAlloc: Allocator + Global API + Vec #108761
Do NOT Review: CoAlloc: Allocator + Global API + Vec #108761
Conversation
2f3933e
to
e91d2d9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read your notes but I am not sure what metadata, exactly, you intend to store and use here. We already have a Layout-based dealloc for all deallocations through our GlobalAlloc, and the Layout is extracted from the type information.
compiler/rustc_arena/src/lib.rs
Outdated
#![allow(incomplete_features)] | ||
#![feature(dropck_eyepatch)] | ||
#![feature(new_uninit)] | ||
#![feature(maybe_uninit_slice)] | ||
#![feature(min_specialization)] | ||
//#![feature(min_specialization)] | ||
#![feature(specialization)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Full specialization is unsound, so this will probably not be accepted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thx. Worked around with min_specialization
.
compiler/rustc_ast/src/ast.rs
Outdated
static_assert_size!(Block, 32); | ||
static_assert_size!(Expr, 72); | ||
static_assert_size!(ExprKind, 40); | ||
static_assert_size!(Fn, 152); | ||
static_assert_size!(Block, 32 + mem::size_of::<<Global as Allocator>::CoAllocMeta>()); | ||
static_assert_size!(Expr, 72 + mem::size_of::<<Global as Allocator>::CoAllocMeta>()); | ||
static_assert_size!(ExprKind, 40 + mem::size_of::<<Global as Allocator>::CoAllocMeta>()); | ||
static_assert_size!(Fn, 152 + 2 * mem::size_of::<<Global as Allocator>::CoAllocMeta>()); | ||
static_assert_size!(ForeignItem, 96); | ||
static_assert_size!(ForeignItemKind, 24); | ||
static_assert_size!(GenericArg, 24); | ||
static_assert_size!(GenericBound, 56); | ||
static_assert_size!(Generics, 40); | ||
static_assert_size!(Impl, 136); | ||
static_assert_size!(Item, 136); | ||
static_assert_size!(ItemKind, 64); | ||
static_assert_size!(GenericBound, 56 + mem::size_of::<<Global as Allocator>::CoAllocMeta>()); | ||
static_assert_size!(Generics, 40 + 2 * mem::size_of::<<Global as Allocator>::CoAllocMeta>()); | ||
static_assert_size!(Impl, 136 + 3 * mem::size_of::<<Global as Allocator>::CoAllocMeta>()); | ||
static_assert_size!(Item, 136 + 3 * mem::size_of::<<Global as Allocator>::CoAllocMeta>()); | ||
static_assert_size!(ItemKind, 64 + 3 * mem::size_of::<<Global as Allocator>::CoAllocMeta>()); | ||
static_assert_size!(LitKind, 24); | ||
static_assert_size!(Local, 72); | ||
static_assert_size!(MetaItemLit, 40); | ||
static_assert_size!(Param, 40); | ||
static_assert_size!(Pat, 72); | ||
static_assert_size!(Pat, 72 + mem::size_of::<<Global as Allocator>::CoAllocMeta>()); | ||
static_assert_size!(Path, 24); | ||
static_assert_size!(PathSegment, 24); | ||
static_assert_size!(PatKind, 48); | ||
static_assert_size!(PatKind, 48 + mem::size_of::<<Global as Allocator>::CoAllocMeta>()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There would have to be significant performance benefits to justify growing this many essential types by this much. Improving on allocation patterns significantly could do that, but this may prove somewhat optimistic.
1906269
to
785152c
Compare
☔ The latest upstream changes (presumably #108471) made this pull request unmergeable. Please resolve the merge conflicts. |
9d59062
to
8a4b19e
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
3ab0a8b
to
810e0f2
Compare
This comment has been minimized.
This comment has been minimized.
810e0f2
to
7090722
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #108944) made this pull request unmergeable. Please resolve the merge conflicts. |
2b4a82c
to
df4e56e
Compare
This comment has been minimized.
This comment has been minimized.
@peter-kehl any updates on this? thanks |
db46df9
to
590acba
Compare
27976f4
to
2d08dec
Compare
This comment has been minimized.
This comment has been minimized.
FYI Currently this causes an ICE (by adding
Reported as #118628. |
2d08dec
to
ad541b4
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
☔ The latest upstream changes (presumably #118457) made this pull request unmergeable. Please resolve the merge conflicts. |
4c22b47
to
56e6e74
Compare
This comment has been minimized.
This comment has been minimized.
56e6e74
to
698c95e
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
451f15d
to
85f72e3
Compare
The job Click to see the possible cause of the failure (guessed by this bot)
|
☔ The latest upstream changes (presumably #118763) made this pull request unmergeable. Please resolve the merge conflicts. |
@peter-kehl do we still need this? thanks |
Closing this as it's inactive for 7 months |
Work in progress. Instead of #108274.
r? peter-kehl