> On Nov 1, 2017, at 8:05 PM, David Sweeris via swift-dev <swift-dev@swift.org> > wrote: > > In "SILFunction.h", line 171, it says, > /// The function's set of semantics attributes. > /// > /// TODO: Why is this using a std::string? Why don't we use uniqued > /// StringRefs? > llvm::SmallVector<std::string, 1> SemanticsAttrSet; > > As an outside contributor, seeing as how I have no idea why a std::string was > used instead of a uniqued StringRef (or TBH, even what it means to "unique" a > StringRef in this context), is this the sort of thing I should not worry > about "fixing"?
The TODO is proposing is suggesting that we might want to reduce the overall memory usage of semantics attributes. std::string owns a unique copy of its string data, so if the same semantics attribute is added to many different SILFunctions, you can end up with the same data duplicated many different times. StringRef is basically a raw pointer to string data stored elsewhere, which means two things: First, to actually be a memory savings, we want that to be same string data for every use of the same attribute. We can do that by just having a set keyed by strings: you look up the string; if it's there, you use the key data already stored in the set; otherwise, you insert the string into the set. That's all we mean by uniquing; it's also often called "interning" in the context of strings. Second, to be safe, that string set needs to outlive the SILFunction. The obvious way to do that is to just store it on the SILModule. But... it turns out that we already have a uniquing set for short string data that outlives the SILFunction, and that's the Identifier table in the ASTContext. So the easy change here would be to make this a SmallVector of Identifiers and just look up the attribute string in the Identifier table when adding it. I express no opinions about whether this is actually a worthwhile optimization. I suspect that the more important memory savings would be to find a way to not store a full SmallVector on every SILFunction, because while the standard library has quite a few such functions, they're far from the majority case even there, and the proportion is basically 0 when you're talking about user code. Combining these two ideas, we could just switch over to using an llvm::TinyPtrVector<Identifier>. John.
_______________________________________________ swift-dev mailing list swift-dev@swift.org https://lists.swift.org/mailman/listinfo/swift-dev
