Skip to content
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

Merged
merged 1 commit into from
Jun 30, 2021
Merged

fix excess array object padding #41287

merged 1 commit into from
Jun 30, 2021

Conversation

JeffBezanson
Copy link
Sponsor Member

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:

julia> f() = for _ in 1:10^6; []; end

julia> @btime f()
  19.540 ms (1000000 allocations: 76.29 MiB)

after:

julia> @btime f()
  18.020 ms (1000000 allocations: 45.78 MiB)

@JeffBezanson JeffBezanson added performance Must go faster domain:arrays [a, r, r, a, y, s] labels Jun 20, 2021
@yuyichao
Copy link
Contributor

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.

@JeffBezanson
Copy link
Sponsor Member Author

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?

@JeffBezanson
Copy link
Sponsor Member Author

Ah, currently we only actually get 64 alignment if the object is not pool-allocated, or if the data is bigger than ARRAY_INLINE_NBYTES.

@JeffBezanson
Copy link
Sponsor Member Author

Ok I think this is right.

@@ -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 []"
Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@oscardssmith
Copy link
Member

Why is this backport elgible?

@JeffBezanson
Copy link
Sponsor Member Author

It just saves memory 🤷

@quinnj
Copy link
Member

quinnj commented Jun 21, 2021

Yeah, I'd be pretty concerned if code was relying on the exact array object representation itself (i.e. jl_array_t) in memory. Though I won't deny having considered such code myself..... 😅

@JeffBezanson
Copy link
Sponsor Member Author

The plot thickens! This also reduces the size of the system image by a little over 3%, about 5Mb 🎉

@JeffBezanson
Copy link
Sponsor Member Author

Found a broken test as well. Storing to a Ptr{Union{Int8,UInt8}} actually writes a reference to a boxed object. We should perhaps change that to match unboxed union layouts.

@JeffBezanson
Copy link
Sponsor Member Author

Ok trying a simpler version where there is just a threshold for when to use larger alignment.

src/staticdata.c Outdated Show resolved Hide resolved
@JeffBezanson JeffBezanson merged commit 61572a6 into master Jun 30, 2021
@JeffBezanson JeffBezanson deleted the jb/arrayalign branch June 30, 2021 02:29
KristofferC pushed a commit that referenced this pull request Jun 30, 2021
johanmon pushed a commit to johanmon/julia that referenced this pull request Jul 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:arrays [a, r, r, a, y, s] performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants