[ntuple] return const char* instead of std::string from RColumnElemen…

…t::GetTypeName This doesn't impose a potential allocation (or the use of std::string) on the caller unnecessarily
root-project · Oct 16, 2024 · 67f2067 · 67f2067
1 parent 2f7462f
commit 67f2067
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 9 deletions.
diff --git a/tree/ntuple/v7/inc/ROOT/RColumnElementBase.hxx b/tree/ntuple/v7/inc/ROOT/RColumnElementBase.hxx
@@ -73,7 +73,7 @@ public:
    /// If CppT == void, use the default C++ type for the given column type
    template <typename CppT = void>
    static std::unique_ptr<RColumnElementBase> Generate(EColumnType type);
-   static std::string GetTypeName(EColumnType type);
+   static const char *GetTypeName(EColumnType type);
    /// Most types have a fixed on-disk bit width. Some low-precision column types
    /// have a range of possible bit widths. Return the minimum and maximum allowed
    /// bit size per type.

diff --git a/tree/ntuple/v7/src/RColumnElement.cxx b/tree/ntuple/v7/src/RColumnElement.cxx
@@ -67,7 +67,7 @@ ROOT::Experimental::Internal::RColumnElementBase::GetValidBitRange(EColumnType t
    return std::make_pair(0, 0);
 }
 
-std::string ROOT::Experimental::Internal::RColumnElementBase::GetTypeName(EColumnType type)
+const char *ROOT::Experimental::Internal::RColumnElementBase::GetTypeName(EColumnType type)
 {
    switch (type) {
    case EColumnType::kIndex64: return "Index64";

diff --git a/tree/ntuple/v7/src/RNTupleDescriptorFmt.cxx b/tree/ntuple/v7/src/RNTupleDescriptorFmt.cxx
@@ -194,7 +194,7 @@ void ROOT::Experimental::RNTupleDescriptor::PrintInfo(std::ostream &output) cons
       std::string nameAndType = std::string("  ") + col.fFieldName + " [#" + std::to_string(col.fColumnIndex);
       if (col.fRepresentationIndex > 0)
          nameAndType += " / R." + std::to_string(col.fRepresentationIndex);
-      nameAndType += "]  --  " + Internal::RColumnElementBase::GetTypeName(col.fType);
+      nameAndType += "]  --  " + std::string{Internal::RColumnElementBase::GetTypeName(col.fType)};
       std::string id = std::string("{id:") + std::to_string(col.fLogicalColumnId) + "}";
       if (col.fLogicalColumnId != col.fPhysicalColumnId)
          id += " --alias--> " + std::to_string(col.fPhysicalColumnId);

diff --git a/tree/ntupleutil/v7/src/RNTupleInspector.cxx b/tree/ntupleutil/v7/src/RNTupleInspector.cxx
@@ -300,8 +300,8 @@ ROOT::Experimental::RNTupleInspector::GetColumnTypeInfoAsHist(ROOT::Experimental
       default: throw RException(R__FAIL("Unknown histogram type"));
       }
 
-      hist->AddBinContent(
-         hist->GetXaxis()->FindBin(Internal::RColumnElementBase::GetTypeName(colInfo.GetType()).c_str()), data);
+      hist->AddBinContent(hist->GetXaxis()->FindBin(Internal::RColumnElementBase::GetTypeName(colInfo.GetType())),
+                          data);
    }
 
    return hist;
@@ -322,9 +322,10 @@ ROOT::Experimental::RNTupleInspector::GetPageSizeDistribution(ROOT::Experimental
                                                               std::string histName, std::string histTitle, size_t nBins)
 {
    if (histName.empty())
-      histName = "pageSizeHistCol" + Internal::RColumnElementBase::GetTypeName(colType);
+      histName = "pageSizeHistCol" + std::string{Internal::RColumnElementBase::GetTypeName(colType)};
    if (histTitle.empty())
-      histTitle = "Page size distribution for columns with type " + Internal::RColumnElementBase::GetTypeName(colType);
+      histTitle = "Page size distribution for columns with type " +
+                  std::string{Internal::RColumnElementBase::GetTypeName(colType)};
 
    auto perTypeHist = GetPageSizeDistribution({colType}, histName, histTitle, nBins);
 
@@ -414,8 +415,8 @@ std::unique_ptr<THStack> ROOT::Experimental::RNTupleInspector::GetPageSizeDistri
 
    for (const auto &[colType, pageSizesForColType] : pageSizes) {
       auto hist = std::make_unique<TH1D>(
-         TString::Format("%s%s", histName.c_str(), Internal::RColumnElementBase::GetTypeName(colType).c_str()),
-         Internal::RColumnElementBase::GetTypeName(colType).c_str(), nBins, histMin,
+         TString::Format("%s%s", histName.c_str(), Internal::RColumnElementBase::GetTypeName(colType)),
+         Internal::RColumnElementBase::GetTypeName(colType), nBins, histMin,
          histMax + ((histMax - histMin) / static_cast<double>(nBins)));
 
       for (const auto pageSize : pageSizesForColType) {