-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
fix excess array object padding #41287
Conversation
There does seem to be some extra alignments (there should be only one alignment per branch) but it is added intentionally in #15139. The default allocated arrays are meant to actually have 64byte alignment. |
I see. That PR mentions only doing it for large objects, but it actually aligns to 64 unconditionally. Is there a size cutoff we could use? |
Ah, currently we only actually get 64 alignment if the object is not pool-allocated, or if the data is bigger than |
5ed5983
to
29db610
Compare
Ok I think this is right. |
29db610
to
1767dd9
Compare
test/cmdlineargs.jl
Outdated
@@ -324,7 +324,7 @@ let exename = `$(Base.julia_cmd()) --startup-file=no --color=no` | |||
rm(memfile) | |||
@test popfirst!(got) == " 0 g(x) = x + 123456" | |||
@test popfirst!(got) == " - function f(x)" | |||
@test popfirst!(got) == " 80 []" | |||
@test popfirst!(got) == " 48 []" |
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.
🎉
1767dd9
to
5a695b6
Compare
Why is this backport elgible? |
It just saves memory 🤷 |
Yeah, I'd be pretty concerned if code was relying on the exact array object representation itself (i.e. |
5a695b6
to
01a7d27
Compare
The plot thickens! This also reduces the size of the system image by a little over 3%, about 5Mb 🎉 |
01a7d27
to
caee0ec
Compare
Found a broken test as well. Storing to a |
caee0ec
to
e0895a5
Compare
Ok trying a simpler version where there is just a threshold for when to use larger alignment. |
e0895a5
to
e209616
Compare
e209616
to
317c4e4
Compare
(cherry picked from commit 61572a6)
We seem to be wasting space currently by excessively padding Array objects. Even though array data is only aligned to 16 bytes, we for some reason align the object size to 64 bytes, then add more (un-aligned) space anyway. Hence the smallest array object is 80 bytes when it could be 48 (or 96 -> 64 when there are some elements). I can't think of a reason to do that, but maybe I'm missing something.
before:
after: