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.

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

Reduce SymbolStringPtr copies. #55576

Open
lhames opened this issue May 19, 2022 · 4 comments
Open

Reduce SymbolStringPtr copies. #55576

lhames opened this issue May 19, 2022 · 4 comments
Assignees
Labels

Comments

@lhames
Copy link
Contributor

lhames commented May 19, 2022

ExecutionSession::lookup currently makes several copies of the SymbolStringPtrs involved in the lookup. As of f9aef47 they're present in at least the AsynchronousSymbolQuery and the InProgressLookupSet::LookupSet member. They're also at least partially duplicated in the DefGeneratorCandidates and DefGeneratorNonCandidates sets of InProgressLookupSet.

If we introduced a new SymbolStringWeakPtr class (and probably a weakly-referencing version of LookupSet built on top of it), we could probably avoid many of these copies.

Note: Naming the weakly-referencing LookupSet will be an interesting exercise: We need to disambiguate weak reference (in the shared-ptr sense) of symbol strings and weak reference (in the linker sense) of symbols. Maybe SymbolStringUnsafePtr would be better? Or SymbolStringUnretainedPtr?

@lhames lhames added the orcjit label May 19, 2022
@lhames lhames self-assigned this May 19, 2022
@llvmbot
Copy link
Collaborator

llvmbot commented May 19, 2022

@llvm/issue-subscribers-orcjit

@lhames
Copy link
Contributor Author

lhames commented May 19, 2022

More potentially unnecessary SymbolStringPtr copies: EPCDynamicLibrarySearchGenerator::tryToGenerate is copying SymbolStringPtrs to build the lookup set that it will pass to ExecutorProcessControl::lookupSymbols, but the lookupSymbols method could probably be rewritten to use StringRefs instead. If that's the case we could save the copies here.

@lhames lhames changed the title Add SymbolStringWeakPtr, use it to avoid SymbolStringPtr copies during lookup. Reduce SymbolStringPtr copies. May 19, 2022
@lhames
Copy link
Contributor Author

lhames commented May 19, 2022

JITDylib's MaterializingInfosMap and UnmaterializedInfosMap members could be keyed on SymbolStringWeakPtr -- their entries should be a subset of the entries in the main symbol table.

@lhames
Copy link
Contributor Author

lhames commented May 19, 2022

We could probably use SymbolStringWeakPtrs in the symbol-dependence graph too. That would be a big win: copies in the dep graph will be a relatively common operation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants