Minor improvements to the use of StringMap/StringSet #19292
Conversation
An llvm::StringSet is "just" an llvm::StringMap where the values are ignored, so the 'keys()' range already does what we need. No functionality change.
|
Reviewers are mostly tagged to check that I didn't make a stupid mistake, particularly in components I don't spend much time in. @swift-ci Please test |
|
@swift-ci Please test source compatibility |
|
@swift-ci Please test compiler performance |
|
@nkcsgexi: There are a bunch in SourceKit I was not confident about, so you may want to do a pass yourself, searching for "StringMap<" or "StringSet<". |
|
👍 for IRGen change |
Build comment file:Compilation-performance test failed |
|
Build failed |
|
Build failed |
jrose-apple
force-pushed
the
get-out-the-map
branch
from
e600143 to
c1ad676
Compare
|
@swift-ci please test |
|
@swift-ci Please test compiler performance |
|
@swift-ci Please test source compatibility |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
jrose-apple
force-pushed
the
get-out-the-map
branch
from
c1ad676 to
4dcda10
Compare
|
@swift-ci Please test |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
I can't win with this one. I guess I should have been more careful. |
That is, where we aren't using the value for anything. No functionality change.
StringMap always copies its strings into its own storage. A DenseMap of StringRefs has the same caveats as any other use of StringRef, but in the cases I've changed the string has very clear ownership that outlives the map. No functionality change, but should reduce memory usage and malloc traffic a little.
jrose-apple
force-pushed
the
get-out-the-map
branch
from
4dcda10 to
71760bc
Compare
|
@swift-ci Please test |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@swift-ci Please test source compatibility |
| return false; | ||
| seen = true; | ||
| return true; | ||
| return duplicates.insert(dependency.RawPath).second; |
keys()