[llvm-profdata] Use string ref for FunctionSamplesMap

[huangjd]

Summary

Use string ref instead of std::string for the key type of FunctionSamplesMap.

In all usages of FunctionSamplesMap, the key type (function name) always comes from existing StringRef (which is backed by memory somewhere else) or from a string which is a part of the sample profile. There is no need store a copy of the function name here,

[huangjd]

rebase

[davidxl]

is there way to make sure the lifetime of underlying strings out lasts the SampleMap?

[davidxl]

For testing purpose, run the reader (via compiler and llvm-profdata) built with address sanitizer will help.

[huangjd]

In D150334#4335942, @davidxl wrote:

For testing purpose, run the reader (via compiler and llvm-profdata) built with address sanitizer will help.

How do you build it with address sanitizer? Is this a LLVM component?

[davidxl]

see https://llvm.org/docs/CMake.html about LLVM_USE_SANTIZER:STRING which passes -fsanitize= .. option. Here STRING should be Address.

[hoy]

In D150334#4335933, @davidxl wrote:

is there way to make sure the lifetime of underlying strings out lasts the SampleMap?

It should be ok, as the compiler and llvm-profdata both keep the underlying sample reader alive to the last minute. llvm-profgen should be fine too.

Wondering if Google's create-profile tool ever uses this DS.

[wenlei]

llvm/lib/ProfileData/SampleProf.cpp

517

This is now creating a StringRef out of a temp string OrigChildContext.getName().str().

[hoy]

llvm/lib/ProfileData/SampleProf.cpp

517

Good catch! Should just use OrigChildContext.getName().

[wenlei]

llvm/lib/ProfileData/SampleProf.cpp

517

This is why it'd be safer to run through asan for this kind of change.

[huangjd]

Updated a few missed std::string -> StringRef

[huangjd]

What is the highest level of undefined behavior check that can be reasonably specified? I rebuilt the project with -DLLVM_USE_SANITIZER=Address and confirmed there is no new failed tests, and searched through all usage of FunctionSampleMap

[davidxl]

lgtm.

llvm/include/llvm/ProfileData/SampleProf.h

 }
 
 class FunctionSamples;
 class SampleProfileReaderItaniumRemapper;
 
 using BodySampleMap = std::map<LineLocation, SampleRecord>;
 // NOTE: Using a StringMap here makes parsed profiles consume around 17% more
 // memory, which is *very* significant for large profiles.
-using FunctionSamplesMap = std::map<std::string, FunctionSamples, std::less<>>;
+using FunctionSamplesMap = std::map<StringRef, FunctionSamples, std::less<>>;
 using CallsiteSampleMap = std::map<LineLocation, FunctionSamplesMap>;
 using LocToLocMap =
     std::unordered_map<LineLocation, LineLocation, LineLocationHash>;
 
 /// Representation of the samples collected for a function.
 ///
 /// This data structure contains all the collected samples for the body
 /// of a function. Each sample corresponds to a LineLocation instance

llvm/lib/ProfileData/SampleProf.cpp

for (auto &It : Node.AllChildFrames) {
     if (!ChildProfile)
       continue;
     SampleContext OrigChildContext = ChildProfile->getContext();
     // Reset the child context to be contextless.
     ChildProfile->getContext().setName(OrigChildContext.getName());
     if (NodeProfile) {
       // Add child profile to the callsite profile map.
       auto &SamplesMap = NodeProfile->functionSamplesAt(ChildNode.CallSiteLoc);
-      SamplesMap.emplace(OrigChildContext.getName().str(), *ChildProfile);
+      SamplesMap.emplace(OrigChildContext.getName(), *ChildProfile);
       NodeProfile->addTotalSamples(ChildProfile->getTotalSamples());
       // Remove the corresponding body sample for the callsite and update the
       // total weight.
       auto Count = NodeProfile->removeCalledTargetAndBodySample(
           ChildNode.CallSiteLoc.LineOffset, ChildNode.CallSiteLoc.Discriminator,
           OrigChildContext.getName());
       NodeProfile->removeTotalSamples(Count);
     }
 
     // Separate child profile to be a standalone profile, if the current parent
     // profile doesn't exist. This is a duplicating operation when the child
     // profile is already incorporated into the parent which is still useful and
     // thus done optionally. It is seen that duplicating context profiles into
     // base profiles improves the code quality for thinlto build by allowing a
     // profile in the prelink phase for to-be-fully-inlined functions.
     if (!NodeProfile) {
       ProfileMap[ChildProfile->getContext()].merge(*ChildProfile);
     } else if (GenerateMergedBaseProfiles) {
       ProfileMap[ChildProfile->getContext()].merge(*ChildProfile);
       auto &SamplesMap = NodeProfile->functionSamplesAt(ChildNode.CallSiteLoc);
-      SamplesMap[ChildProfile->getName().str()].getContext().setAttribute(
+      SamplesMap[ChildProfile->getName()].getContext().setAttribute(
           ContextDuplicatedIntoBase);
     }
 
     // Remove the original child profile.
     ProfileMap.erase(OrigChildContext);
   }
 }
 
 void ProfileConverter::convertCSProfiles() { convertCSProfiles(RootFrame); }

llvm/lib/ProfileData/SampleProfReader.cpp

if ((*LineIt)[0] != ' ') {
       Discriminator &= getDiscriminatorMask();
 
       while (InlineStack.size() > Depth) {
         InlineStack.pop_back();
       }
       switch (LineTy) {
       case LineType::CallSiteProfile: {
         FunctionSamples &FSamples = InlineStack.back()->functionSamplesAt(
-            LineLocation(LineOffset, Discriminator))[std::string(FName)];
+            LineLocation(LineOffset, Discriminator))[FName];
         FSamples.setName(FName);
         MergeResult(Result, FSamples.addTotalSamples(NumSamples));
         InlineStack.push_back(&FSamples);
         DepthMetadata = 0;
         break;
       }
       case LineType::BodyProfile: {
         while (InlineStack.size() > Depth) {
for (uint32_t J = 0; J < *NumCallsites; ++J) {
     auto FName(readStringFromTable());
     if (std::error_code EC = FName.getError())
       return EC;
 
     // Here we handle FS discriminators:
     uint32_t DiscriminatorVal = (*Discriminator) & getDiscriminatorMask();
 
     FunctionSamples &CalleeProfile = FProfile.functionSamplesAt(
-        LineLocation(*LineOffset, DiscriminatorVal))[std::string(*FName)];
+        LineLocation(*LineOffset, DiscriminatorVal))[*FName];
     CalleeProfile.setName(*FName);
     if (std::error_code EC = readProfile(CalleeProfile))
       return EC;
   }
 
   return sampleprof_error::success;
 }
 
if (!ProfileIsCS) {
         if (std::error_code EC = FContext.getError())
           return EC;
 
         FunctionSamples *CalleeProfile = nullptr;
         if (FProfile) {
           CalleeProfile = const_cast<FunctionSamples *>(
               &FProfile->functionSamplesAt(LineLocation(
                   *LineOffset,
-                  *Discriminator))[std::string(FContext.get().getName())]);
+                  *Discriminator))[FContext->getName()]);
         }
         if (std::error_code EC =
                 readFuncMetadata(ProfileHasAttribute, CalleeProfile))
           return EC;
       }
     }
   }
 
std::error_code SampleProfileReaderGCC::readOneFunctionProfile(
   } else {
     // Otherwise, we are reading an inlined instance. The top of the
     // inline stack contains the profile of the caller. Insert this
     // callee in the caller's CallsiteMap.
     FunctionSamples *CallerProfile = InlineStack.front();
     uint32_t LineOffset = Offset >> 16;
     uint32_t Discriminator = Offset & 0xffff;
     FProfile = &CallerProfile->functionSamplesAt(
-        LineLocation(LineOffset, Discriminator))[std::string(Name)];
+        LineLocation(LineOffset, Discriminator))[Name];
   }
   FProfile->setName(Name);
 
   for (uint32_t I = 0; I < NumPosCounts; ++I) {
     uint32_t Offset;
     if (!GcovBuffer.readInt(Offset))
       return sampleprof_error::truncated;
 

llvm/tools/llvm-profdata/llvm-profdata.cpp

for (const auto &BodySample : Samples.getBodySamples()) {
     }
   }
   for (const auto &CallsiteSamples : Samples.getCallsiteSamples()) {
     sampleprof::FunctionSamplesMap &Target =
         Result.functionSamplesAt(CallsiteSamples.first);
     for (const auto &Callsite : CallsiteSamples.second) {
       sampleprof::FunctionSamples Remapped =
           remapSamples(Callsite.second, Remapper, Error);
-      MergeResult(Error,
-                  Target[std::string(Remapped.getName())].merge(Remapped));
+      MergeResult(Error, Target[Remapped.getName()].merge(Remapped));
     }
   }
   return Result;
 }
 
 static sampleprof::SampleProfileFormat FormatMap[] = {
     sampleprof::SPF_None,
     sampleprof::SPF_Text,

llvm/tools/llvm-profgen/ProfileGenerator.cpp

FunctionSamples &ProfileGenerator::getLeafProfileAndAddTotalSamples(
 
   for (size_t I = 1; I < FrameVec.size(); I++) {
     LineLocation Callsite(
         FrameVec[I - 1].Location.LineOffset,
         getBaseDiscriminator(FrameVec[I - 1].Location.Discriminator));
     FunctionSamplesMap &SamplesMap =
         FunctionProfile->functionSamplesAt(Callsite);
     auto Ret =
-        SamplesMap.emplace(FrameVec[I].FuncName.str(), FunctionSamples());
+        SamplesMap.emplace(FrameVec[I].FuncName, FunctionSamples());
     if (Ret.second) {
       SampleContext Context(FrameVec[I].FuncName);
       Ret.first->second.setContext(Context);
     }
     FunctionProfile = &Ret.first->second;
     FunctionProfile->addTotalSamples(Count);
     if (Binary->usePseudoProbes()) {
       const auto *FuncDesc = Binary->getFuncDescForGUID(

llvm/unittests/ProfileData/SampleProfTest.cpp

void testRoundTrip(SampleProfileFormat Format, bool Remap, bool UseMD5) {
     FooSamples.addBodySamples(2, 0, 600);
     FooSamples.addBodySamples(4, 0, 60000);
     FooSamples.addBodySamples(8, 0, 60351);
     FooSamples.addBodySamples(10, 0, 605);
 
     // Add inline instance with name "_Z3gooi".
     StringRef GooName("_Z3gooi");
     auto &GooSamples =
-        FooSamples.functionSamplesAt(LineLocation(7, 0))[GooName.str()];
+        FooSamples.functionSamplesAt(LineLocation(7, 0))[GooName];
     GooSamples.setName(GooName);
     GooSamples.addTotalSamples(502);
     GooSamples.addBodySamples(3, 0, 502);
 
     // Add inline instance with name "_Z3hooi".
     StringRef HooName("_Z3hooi");
     auto &HooSamples =
-        GooSamples.functionSamplesAt(LineLocation(9, 0))[HooName.str()];
+        GooSamples.functionSamplesAt(LineLocation(9, 0))[HooName];
     HooSamples.setName(HooName);
     HooSamples.addTotalSamples(317);
     HooSamples.addBodySamples(4, 0, 317);
 
     StringRef BarName("_Z3bari");
     FunctionSamples BarSamples;
     BarSamples.setName(BarName);
     BarSamples.addTotalSamples(20301);