alexcrichton opened PR #3236 from less-parse
to main
:
One of the slightly-more-expensive parts of creating a
CodeMemory
at
this time is how symbol names in the object are parsed to determine
whether they're for a defined function or a trampoline. Given the rigid
structure of the object file, however, we don't actually need to do
parsing and instead we can guess theSymbolIndex
associated with each
desired function. This means we can eschew parsing altogether and we
never actually need to guess the name of the symbol.This commit also defers allocation of trampoline/function maps out of
CodeMemory
into the instantiation portions since that's already
rebuilding the maps and there's no need to build them twice.<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
alexcrichton edited PR #3236 from less-parse
to main
:
One of the slightly-more-expensive parts of creating a
CodeMemory
at
this time is how symbol names in the object are parsed to determine
whether they're for a defined function or a trampoline. Given the rigid
structure of the object file, however, we don't actually need to do
parsing and instead we can guess theSymbolIndex
associated with each
desired function. This means we can eschew parsing altogether and we
never actually need to guess the name of the symbol.This commit also defers allocation of trampoline/function maps out of
CodeMemory
into the instantiation portions since that's already
rebuilding the maps and there's no need to build them twice.cc #3230
<!--
Please ensure that the following steps are all taken care of before submitting
the PR.
[ ] This has been discussed in issue #..., or if not, please tell us why
here.[ ] A short description of what this does, why it is needed; if the
description becomes long, the matter should probably be discussed in an issue
first.[ ] This PR contains test cases, if meaningful.
- [ ] A reviewer from the core maintainer team has been assigned for this PR.
If you don't know who could review this, please indicate so. The list of
suggested reviewers on the right can help you.Please ensure all communication adheres to the code of conduct.
-->
alexcrichton requested cfallin for a review on PR #3236.
alexcrichton updated PR #3236 from less-parse
to main
.
alexcrichton updated PR #3236 from less-parse
to main
.
alexcrichton updated PR #3236 from less-parse
to main
.
cfallin submitted PR review.
cfallin submitted PR review.
cfallin created PR review comment:
This mapping feels pretty brittle, and likely to break accidentally with a future change. Is there a way that we could export a helper from the
object
crate (or factor into a third crate if that's an unwanted dep) that turns this pseudo-specified mapping into a more solid thing?
cfallin created PR review comment:
s/is/are/ or s/trampolines/a trampoline/ (sorry for extreme grammar nitpick)
cfallin created PR review comment:
There might be a slightly better name for this -- it seems like an escape analysis of sorts, so maybe "escaped functions"? An export is a sort of escape, and so is creation of a reference or insertion into a table.
cfallin created PR review comment:
(On reading further down, I do see the debug-assert check of this invariant and the related one for trampoline functions, but it still feels somewhat fragile to me as a sort of distributed invariant.)
cfallin submitted PR review.
alexcrichton closed without merge PR #3236.
Last updated: Dec 23 2024 at 13:07 UTC