-
Notifications
You must be signed in to change notification settings - Fork 62
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
LLVM lazy load #1694
LLVM lazy load #1694
Conversation
93cfc3c
to
55c34fc
Compare
-- | Look up the 'L.Define' for a symbol defined in an 'LLVMModule' | ||
lookupFunctionDef :: LLVMModule arch -> String -> Maybe L.Define | ||
lookupFunctionDef lm nm = | ||
find ((fromString nm ==) . L.defName) $ L.modDefines $ modAST lm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've gone from a Map
lookup to a list lookup, which seems somewhat unfortunate. Could this be a performance bottleneck in practice?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure; I don't have a good sense of how hot this lookup function is. I'd be surprised if it is a major bottleneck, but it's a shame to be going backward in terms of algorithmic complexity.
Ideally, the llvm-pretty library, or some intermediate layer would provide better lookup functionality for this stuff so it doesn't have to be redone everywhere downstream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eddywestbrook, would you be willing to take a quick look at these heapster-releated changes and give a sense of whether this is a concern for you?
55c34fc
to
4f4e1d7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I'm not sure what happened with the MacOS / saw-remote-api test, but given it passed on other platforms I'm willing to write it off as a transient failure. |
Pull in Crucible changes that enable lazy loading of Crucible CFGs from LLVM.