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.

Sign up for GitHub

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

Jump to bottom

Move JITLink from StringRefs to SymbolStringPtrs #55533

Open
weliveindetail opened this issue May 17, 2022 · 11 comments
Open

Move JITLink from StringRefs to SymbolStringPtrs #55533

weliveindetail opened this issue May 17, 2022 · 11 comments
Assignees
@jaredwy
Labels
good first issue https://github.com/llvm/llvm-project/contribute jitlink orcjit

Comments

@weliveindetail
Copy link
Member

JITLink is using the lllvm::StringRef type in many places where llvm::orc::SymbolStringPtr would result in better performance and memory footprint, e.g.

llvm-project/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h

Lines 648 to 649 in 7748055

// FIXME: A char* or SymbolStringPtr may pack better.
StringRef Name;

In such cases the string should be interned in a llvm::orc::SymbolStringPool and referenced via llvm::orc::SymbolStringPtr.
@weliveindetail weliveindetail added good first issue https://github.com/llvm/llvm-project/contribute orcjit jitlink labels May 17, 2022
@llvmbot
Copy link
Member

llvmbot commented May 17, 2022

@llvm/issue-subscribers-orcjit

@llvmbot
Copy link
Member

llvmbot commented May 17, 2022

@llvm/issue-subscribers-good-first-issue

@llvmbot
Copy link
Member

llvmbot commented May 17, 2022

@llvm/issue-subscribers-jitlink

@moazin
Copy link

moazin commented May 28, 2022

I'm gonna try and fix this. Will report back on progress soon.

@weliveindetail
Copy link
Member Author

@moazin Thanks for looking into this! If you have questions, feel free to reach out in the #JIT Channel in the LLVM Discord: https://discord.gg/5H8JSRe

@jaredwy
Copy link
Contributor

jaredwy commented Aug 31, 2022

There hadn't been any movement on this for a while. So I have a review up for it now. Sorry if I trod on any toes.

@jaredwy jaredwy self-assigned this Aug 31, 2022
@weliveindetail
Copy link
Member Author

Hi Jared, thanks for working on this. Can you maybe link your review here?

@lhames
Copy link
Contributor

lhames commented Sep 3, 2022

@weliveindetail -- The review is at https://reviews.llvm.org/D132988.

@diohabara
Copy link

There don't seem to be any updates to the review at all recently, is it possible to take over this review?

@lhames
Copy link
Contributor

lhames commented Mar 15, 2023

I reached out to @jaredwy and he said that he's almost done with the work and hopes to land it soon.

@miguelraz
Copy link
Contributor

Hey @weliveindetail - I'm very interested in what this PR can help out Julia's rubtime for memory footprint.

Is there anyway I can help push this along?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jaredwy jaredwy

Labels
good first issue https://github.com/llvm/llvm-project/contribute jitlink orcjit
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
7 participants
@lhames @jaredwy @weliveindetail @miguelraz @moazin @diohabara @llvmbot