Skip to content

Conversation

ilovepi
Copy link
Contributor

@ilovepi ilovepi commented Sep 16, 2025

The splitMustacheString function was saving StringRefs that
were already backed by an arena-allocated string. This was
unnecessary work. This change removes the redundant
Ctx.Saver.save() call.

This optimization provides a small but measurable performance
improvement on top of the single-pass tokenizer, most notably
reducing branch misses.

Metric Baseline Optimized Change
Time (ms) 35.77 35.57 -0.56%
Cycles 35.16M 34.91M -0.71%
Instructions 85.77M 85.54M -0.27%
Branch Misses 113.9K 111.9K -1.76%
Cache Misses 237.7K 242.1K +1.85%

Copy link
Contributor Author

ilovepi commented Sep 16, 2025

@llvmbot
Copy link
Member

llvmbot commented Sep 16, 2025

@llvm/pr-subscribers-llvm-support

Author: Paul Kirth (ilovepi)

Changes

The splitMustacheString function was saving StringRefs that
were already backed by an arena-allocated string. This was
unnecessary work. This change removes the redundant
Ctx.Saver.save() call.

This optimization provides a small but measurable performance
improvement on top of the single-pass tokenizer, most notably
reducing branch misses.

Metric Baseline Optimized Change
Time (ms) 35.77 35.57 -0.56%
Cycles 35.16M 34.91M -0.71%
Instructions 85.77M 85.54M -0.27%
Branch Misses 113.9K 111.9K -1.76%
Cache Misses 237.7K 242.1K +1.85%

Full diff: https://github.com/llvm/llvm-project/pull/159197.diff

1 Files Affected:

  • (modified) llvm/lib/Support/Mustache.cpp (+1-1)
diff --git a/llvm/lib/Support/Mustache.cpp b/llvm/lib/Support/Mustache.cpp
index 63798c50f57ee..fcb55c4edd815 100644
--- a/llvm/lib/Support/Mustache.cpp
+++ b/llvm/lib/Support/Mustache.cpp
@@ -52,7 +52,7 @@ static Accessor splitMustacheString(StringRef Str, MustacheContext &Ctx) {
       std::tie(Part, Str) = Str.split('.');
       // Each part of the accessor needs to be saved to the arena
       // to ensure it has a stable address.
-      Tokens.push_back(Ctx.Saver.save(Part.trim()));
+      Tokens.push_back(Part.trim());
     }
   }
   // Now, allocate memory for the array of StringRefs in the arena.

@ilovepi ilovepi force-pushed the users/ilovepi/mustache-accessor-opt branch from b2db0c9 to b68de04 Compare September 22, 2025 17:07
@ilovepi ilovepi force-pushed the users/ilovepi/mustache-tokeniser-opt branch 2 times, most recently from 3ac408e to b929e27 Compare September 22, 2025 17:56
@ilovepi ilovepi force-pushed the users/ilovepi/mustache-accessor-opt branch 2 times, most recently from 1acd655 to bd084ea Compare September 25, 2025 22:12
@ilovepi ilovepi force-pushed the users/ilovepi/mustache-tokeniser-opt branch from b929e27 to 1a9b251 Compare September 25, 2025 22:12
The splitMustacheString function was saving StringRefs that
were already backed by an arena-allocated string. This was
unnecessary work. This change removes the redundant
Ctx.Saver.save() call.

This optimization provides a small but measurable performance
improvement on top of the single-pass tokenizer, most notably
reducing branch misses.

  Metric         | Baseline | Optimized | Change
  -------------- | -------- | --------- | -------
  Time (ms)      | 35.77    | 35.57     | -0.56%
  Cycles         | 35.16M   | 34.91M    | -0.71%
  Instructions   | 85.77M   | 85.54M    | -0.27%
  Branch Misses  | 113.9K   | 111.9K    | -1.76%
  Cache Misses   | 237.7K   | 242.1K    | +1.85%
@ilovepi ilovepi force-pushed the users/ilovepi/mustache-tokeniser-opt branch from 1a9b251 to dba238e Compare September 26, 2025 01:55
@ilovepi ilovepi force-pushed the users/ilovepi/mustache-accessor-opt branch from bd084ea to 735490e Compare September 26, 2025 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants