[NFC] Make public type collection more efficient #7561
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously public type collection worked by collecting all types in the module, classifying their visibility, then picking out all the public types to return. However, visibility classification requires directly finding the public types from the imports and exports in the first place, so the initial step of collecting all the types was unnecessary. Refactor the code so `getPublicHeapTypes` calculates the public types directly from the imports and exports and `classifyTypes` uses those results to classify visibility only when collecting all types from the module anyway.
tlively
commented
Apr 29, 2025
| } | ||
| } | ||
|
|
||
| void setIndices(IndexedHeapTypes& indexedTypes) { |
There was a problem hiding this comment.
Heh, the moved code was so large that it's a smaller diff to show everything else moving around it.
kripken
approved these changes
Apr 29, 2025
src/ir/module-utils.cpp
Outdated
| return types; | ||
| } | ||
|
|
||
| std::vector<HeapType> getPublicHeapTypes(Module& wasm) { | ||
| // We will need to traverse the types used by public types and mark them | ||
| // public as well. |
There was a problem hiding this comment.
"as well" seems odd in the first comment of a function?
There was a problem hiding this comment.
It's referring to the original public types also introduced in this sentence, so I think it makes sense. I will clarify in general, though.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously public type collection worked by collecting all types in the
module, classifying their visibility, then picking out all the public
types to return. However, visibility classification requires directly
finding the public types from the imports and exports in the first
place, so the initial step of collecting all the types was unnecessary.
Refactor the code so
getPublicHeapTypescalculates the public typesdirectly from the imports and exports and
classifyTypesuses thoseresults to classify visibility only when collecting all types from the
module anyway.