[PATCH] D109036: [CSSPGO] Sort function offset table to speed up profile loading.

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/lib/ProfileData/SampleProfReader.cpp:732
   for (uint32_t I = 0; I < *Size; ++I) {
     auto FName(readSampleContextFromTable());
     if (std::error_code EC = FName.getError())
----------------
wenlei wrote:
> FName -> FContext
Fixed.


================
Comment at: llvm/lib/ProfileData/SampleProfReader.cpp:742
+    if (FuncOffsetsOrdered)
+      OrderedFuncOffsets->emplace_back(*FName, *Offset);
   }
----------------
wenlei wrote:
> assert on the order comparing to OrderedFuncOffsets->back()?
The order may not be strictly the lexical order of `SampleContext` for md5 profiles. Md5 profiles may be converted from a string-based profile and the order of the func offset table in the md5 profile will be based on the original string order. However, the caller-callee context order, i.e, the trie tree path order, is still fulfilled. That order is what we want to load profile based on.


================
Comment at: llvm/lib/ProfileData/SampleProfWriter.cpp:44
 
+static cl::opt<bool> SortFuncOffsetTable(
+    "sort-func-offsets", cl::Hidden, cl::init(true),
----------------
wenlei wrote:
> Why do we need a flag? Can we simplify this by always order the function offset table for CSSPGO profile? 
This is mostly for debugging and verification of the unordered path that is kept for being compatible with legacy unordered profiles.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109036/new/

https://reviews.llvm.org/D109036