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

[lldb][Type Completion] Load C++ methods lazily from DWARF #8578

Conversation

Michael137
Copy link

@Michael137 Michael137 commented Apr 16, 2024

Note, this is a no-op when the LLDB setting
plugin.typesystem.clang.experimental-redecl-completion is not set (which is currently the default). So this patch has no effect unless the user explicitly opts into it.

The type-completion rework (aka redecl-completion) implemented in #8222 comes with a large performance penalty, since we now eagerly complete RecordTypes. Completing a RecordType previously unconditionally resolved all member function of said type. With redecl-completion completion, however, this meant we were now pulling in many more definitions than needed. Without redecl-completion, this isn't a problem, since importing method parameters is cheap (they are imported minimally), so we wouldn't notice that we always resolved all member functions.

This patch tries to load methods lazily when in redecl-completion mode. We do this by introducing a new ExternalASTSource::FindExternalVisibleMethods API which Clang calls when it parses a member access expression. The callback into LLDB will do a FindFunctions call for all methods with the method name we're looking for, filters out any mismatches, and lets Clang continue with it's parsing. We still load following methods eagerly:

  1. virtual functions: currently overrides are resolved in CompleteRecordType
  2. operators: currently I couldn't find a point at which Clang can call into LLDB here to facilitate lazy loading
  3. ctors/dtors: same reason as (2)

In our benchmark harness, we saw this patch bring down redecl-completion expression evaluation on-par with top-of-tree expression evaluation.

@Michael137 Michael137 force-pushed the lldb/type-completion-rework/lazy-method-loading-to-next branch 3 times, most recently from 1d50470 to 5ceac14 Compare April 18, 2024 11:32
Note, this is a no-op when the LLDB setting
`plugin.typesystem.clang.experimental-redecl-completion` is
not set (which is currently the default). So this patch has
no affect unless the user explicitly opts into it.

The type-completion rework (aka redecl-completion) implemented in
swiftlang#8222 comes with
a large performance penalty, since we now eagerly complete
`RecordType`s. Completing a `RecordType` previously unconditionally
resolved all member function of said type. With redecl-completion
completion, however, this meant we were now pulling in many
more definitions than needed. Without redecl-completion, this
isn't a problem, since importing method parameters is cheap
(they are imported minimally), so we wouldn't notice that
we always resolved all member functions.

This patch tries to load methods lazily when in redecl-completion
mode. We do this by introducing a new `ExternalASTSource::FindExternalVisibleMethods`
API which Clang parses a member access expression. The callback
into LLDB will do a `FindFunctions` call for all methods with the
method name we're looking for, filters out any mismatches, and
lets Clang continue with it's parsing. We still load following
methods eagerly:
1. virtual functions: currently overrides are resolved in `CompleteRecordType`
2. operators: currently I couldn't find a point at which Clang can call
              into LLDB here to facilitate lazy loading
3. ctors/dtors: same reason as (2)

In our benchmark harness, we saw this patch bring down redecl-completion
expression evaluation on-par with top-of-tree expression evaluation.
@Michael137 Michael137 force-pushed the lldb/type-completion-rework/lazy-method-loading-to-next branch from 2f73e5d to 41c4fca Compare April 18, 2024 16:51
@Michael137 Michael137 merged commit 15fff22 into swiftlang:next Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant