Skip to content

[lldb] Remove use of SwiftASTContext in GetPointerByteSize #6654

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

Conversation

kastiglione
Copy link

@kastiglione kastiglione commented Apr 16, 2023

When TypeSystemSwiftTypeRef instance doesn't correspond to a Module, it should have a Target. Either are sufficient to get the pointer size. This avoids loading and constructing Swift ASTContexts, which can be slow and unnecessary in this case.

@kastiglione
Copy link
Author

@swift-ci test

Comment on lines +2294 to +2295
if (triple.isArch16Bit())
return 2;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we don't need this?

@kastiglione kastiglione merged commit 97f6808 into swift/release/5.9 Apr 17, 2023
@kastiglione kastiglione deleted the dl/lldb-remove-use-of-swiftastcontext-in-getpointerbytesize branch April 17, 2023 18:31
kastiglione added a commit that referenced this pull request Apr 25, 2023
Update `TypeSystemSwiftTypeRef::GetTriple` to make use of an available target.

With this change, `GetNameImporter` can work in more cases, since it only needs a 
triple. And since `GetNameImporter` can be expected to return a `ClangNameImporter` 
instance, the fallback to using Swift ASTContexts can be removed.

This eliminates another case of unnecessarily loading Swift ASTContexts, which incurs a 
one time overhead that should only be paid when required.

Partial follow up to #6654
kastiglione added a commit that referenced this pull request May 5, 2023
When `TypeSystemSwiftTypeRef` instance doesn't correspond to a `Module`, it should have a `Target`. Either are sufficient to get the pointer size. This avoids loading and constructing Swift ASTContexts, which can be slow and unnecessary in this case.

(cherry-picked from commit 97f6808)
kastiglione added a commit that referenced this pull request May 5, 2023
…6786)

When `TypeSystemSwiftTypeRef` instance doesn't correspond to a `Module`, it should have a `Target`. Either are sufficient to get the pointer size. This avoids loading and constructing Swift ASTContexts, which can be slow and unnecessary in this case.

(cherry-picked from commit 97f6808)
kastiglione added a commit that referenced this pull request May 5, 2023
Update `TypeSystemSwiftTypeRef::GetTriple` to make use of an available target.

With this change, `GetNameImporter` can work in more cases, since it only needs a 
triple. And since `GetNameImporter` can be expected to return a `ClangNameImporter` 
instance, the fallback to using Swift ASTContexts can be removed.

This eliminates another case of unnecessarily loading Swift ASTContexts, which incurs a 
one time overhead that should only be paid when required.

Partial follow up to #6654

(cherry-picked from commit f097595)
kastiglione added a commit that referenced this pull request May 5, 2023
…6790)

Update `TypeSystemSwiftTypeRef::GetTriple` to make use of an available target.

With this change, `GetNameImporter` can work in more cases, since it only needs a 
triple. And since `GetNameImporter` can be expected to return a `ClangNameImporter` 
instance, the fallback to using Swift ASTContexts can be removed.

This eliminates another case of unnecessarily loading Swift ASTContexts, which incurs a 
one time overhead that should only be paid when required.

Partial follow up to #6654

(cherry-picked from commit f097595)
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.

2 participants