Encode link_id in tagged linkage #48673
Conversation
vtjnash
added
merge me
|
I never did get around to finishing this. Am I understanding you're OK with the uglification compared to #48279? If so I can focus on this branch. |
On 64-bit, we have enough space to encode (1) the tag, (2) the `depmods` index, and (3) the offset all in a single 64-bit pointer field. This means we don't need the external `link_id` arrays, which reduces the size of many pkgimages by ~5%. On 32-bit, we don't have enough bits to implement this strategy. However, most linkages seem to be against the sysimage, and so by giving that a separate tag we can achieve similar compression because the `link_id` lists will be much shorter.
vtjnash
force-pushed
the
teh/compress2
branch
from
20be3c6
to
7225cd8
Compare
|
I didn't realize it wasn't finished before. It is fixed now. I think you will see the new diff is smaller and more complete than the old diff (and I even combined 2 existing tags, so we are net neutral) 🙂 |
| @@ -4251,7 +4251,7 @@ static jl_cgval_t emit_invoke(jl_codectx_t &ctx, const jl_cgval_t &lival, const | |||
| bool cache_valid = ctx.use_cache; | |||
| bool external = false; | |||
| if (ctx.external_linkage) { | |||
| if (jl_object_in_image((jl_value_t*)codeinst)) { | |||
| if (0 && jl_object_in_image((jl_value_t*)codeinst)) { | |||
There was a problem hiding this comment.
Did you intend to commit this part?
There was a problem hiding this comment.
yes, this code is broken, and I accidentally fixed the code that disabled it
There was a problem hiding this comment.
Add a comment or something then? Looks very weird to have that standing freely.
There was a problem hiding this comment.
Without this code we won't link from package images to the system image. Instead each package image will contain a copy of the native code, so I don't think disabling is an option.
There was a problem hiding this comment.
A comment here and in the other places doing this would indeed have been nice...
vtjnash
force-pushed
the
teh/compress2
branch
from
7225cd8
to
fdfa6ae
Compare
|
A s all size reduction does not seem necessary to backport. And this PR introduces a bunch of dead code so it feels like it needs various follow ups. |
|
What deadcode is introduced here? I accidentally defined some pointers that used to be null, requiring a more explicit code guard, but that didn't change anything observable (indeed, the point of it was to avoid changing anything notable) |
|
You know better than me but it does look weird to have a bunch of |
On 64-bit, we have enough space to encode (1) the tag, (2) the `depmods` index, and (3) the offset all in a single 64-bit pointer field. This means we don't need the external `link_id` arrays, which reduces the size of many pkgimages by ~5%. On 32-bit, we don't have enough bits to implement this strategy. However, most linkages seem to be against the sysimage, and so by giving that a separate tag we can achieve similar compression because the `link_id` lists will be much shorter. Co-authored-by: Tim Holy <tim.holy@gmail.com> (cherry picked from commit 8e3e970)
On 64-bit, we have enough space to encode (1) the tag, (2) the `depmods` index, and (3) the offset all in a single 64-bit pointer field. This means we don't need the external `link_id` arrays, which reduces the size of many pkgimages by ~5%. On 32-bit, we don't have enough bits to implement this strategy. However, most linkages seem to be against the sysimage, and so by giving that a separate tag we can achieve similar compression because the `link_id` lists will be much shorter. Co-authored-by: Tim Holy <tim.holy@gmail.com> (cherry picked from commit 8e3e970)
On 64-bit, we have enough space to encode (1) the tag, (2) the
depmodsindex, and (3) the offset all in a single 64-bit pointer field. This means we don't need the externallink_idarrays, which reduces the size of many pkgimages by ~5%.On 32-bit, we don't have enough bits to implement this strategy. However, most linkages seem to be against the sysimage, and so by giving that a separate tag we can achieve similar compression because the
link_idlists will be much shorter.Fixes: #48218
Closes: #48279