llvm-project icon indicating copy to clipboard operation
llvm-project copied to clipboard

Published 20 hours ago • verified publisher icon llvm

Move JITLink from StringRefs to SymbolStringPtrs

Open weliveindetail opened this issue 3 years ago • 11 comments

JITLink is using the lllvm::StringRef type in many places where llvm::orc::SymbolStringPtr would result in better performance and memory footprint, e.g. https://github.com/llvm/llvm-project/blob/77480556c41fbca36b918323c69cb77f8e02b56c/llvm/include/llvm/ExecutionEngine/JITLink/JITLink.h#L648-L649

In such cases the string should be interned in a llvm::orc::SymbolStringPool and referenced via llvm::orc::SymbolStringPtr.

weliveindetail avatar May 17 '22 12:05 weliveindetail

@llvm/issue-subscribers-orcjit

llvmbot avatar May 17 '22 12:05 llvmbot

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

llvmbot avatar May 17 '22 12:05 llvmbot

@llvm/issue-subscribers-jitlink

llvmbot avatar May 17 '22 12:05 llvmbot

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

moazin avatar May 28 '22 09:05 moazin

@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

weliveindetail avatar Jun 03 '22 12:06 weliveindetail

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 avatar Aug 31 '22 02:08 jaredwy

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

weliveindetail avatar Aug 31 '22 08:08 weliveindetail

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

lhames avatar Sep 03 '22 04:09 lhames

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

diohabara avatar Mar 15 '23 21:03 diohabara

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

lhames avatar Mar 15 '23 23:03 lhames

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?

miguelraz avatar Mar 24 '24 15:03 miguelraz