More benchmarker features #595

rctcwyvrn · 2022-08-03T00:49:06Z

Adds

Loading results from a file
Measuring compile times (via a spi function in Regex)
Comparing compile times and against NSRegularExpression
Saving comparison results as csv
Charts (based on Add option to show benchmark charts #585)

- Compare with NS regular expression - Save comparison results - Save and compare estimated compile times

A bad one because I have no idea how to use Swift Charts

rxwei

📊

rxwei · 2022-08-03T01:21:23Z

Sources/RegexBenchmark/BenchmarkChart.swift

+    BarMark(
+      x: .value("Name", name),
+      y: .value("Normalized runtime", new / old))
+    .foregroundStyle(LinearGradient(
+      colors: [.accentColor, new - old <= 0 ? .green : .yellow],
+      startPoint: .bottom,
+      endPoint: .top))


You could inline this into the ForEach { ... }. I pulled this out to chartBody because of "expression too complex". That doesn't happen with just one bar mark.

rctcwyvrn · 2022-08-03T23:09:09Z

@swift-ci test

rctcwyvrn · 2022-08-03T23:39:16Z

Oh huh what version is the CI on?

/Users/ec2-user/jenkins/workspace/pr-swift-experimental-string-processing-macos/branch-main/swift-experimental-string-processing/Sources/RegexBenchmark/BenchmarkChart.swift:14:8: error: no such module 'Charts'
import Charts
       ^

rxwei · 2022-08-04T01:39:21Z

I don't think the CI runs on macOS 13. Maybe #if canImport(Charts) around the import and around Chart usage.

milseman · 2022-08-04T13:38:30Z

Sources/_StringProcessing/Regex/Core.swift

+      return false
+    }
+  }
+}


This kind of approach looks ok to me. I wonder if we can generalize the interface a little bit to give us some future flexibility without needing to add more SPI over time. For example, we could return a (resilient) result type which we can add metrics to in the future (number of instructions, complexity or star-height, etc). We could also have a (resilient) compilation options parameter, such as the proposed force-recompilation behavior which we could extend with more options like control over optimizations or even targeted engine version (e.g. back deployment)

We may also want to output per-stage metrics, like time spent parsing vs generating bytecode, etc.

rctcwyvrn · 2022-08-05T04:18:30Z

Added setting compiler options to the SPI interface and made it extensible for the future by adding a _RegexInternalAction enum (not sure how to make sure this/the switch are abi stable)
Added some processor metrics code I wrote awhile back and forgot about
Added metrics and tracing enabled as options for the compiler
Using the new additions, added CLI options for turning on metrics/tracing for benchmarker regexes
Added parse time measurements to the benchmarker

rctcwyvrn · 2022-08-10T22:16:23Z

@swift-ci test

rctcwyvrn · 2022-08-10T22:27:08Z

@milseman Could you check that Regex._forceAction in Core.swift matches what you had in mind? Also that it's ABI stable the way it's written right now?

milseman · 2022-08-10T14:28:11Z

Sources/RegexBenchmark/Debug.swift

@@ -6,7 +6,7 @@ extension Benchmark {
    case .whole:
      let result = target.wholeMatch(of: regex)
      if let match = result {
-        if match.0.count > 100 {
+        if match.0.count > 1000 {


Could / should we abstract this check out to a helper method somewhere?

milseman · 2022-08-10T14:28:56Z

Sources/_StringProcessing/Compiler.swift

-    static let disableOptimizations = CompileOptions(rawValue: 1)
-    static let `default`: CompileOptions = []
+@_spi(RegexBenchmark)
+public struct CompileOptions: OptionSet {


Let's underscore the name. Not a huge deal, but it makes it easier if we one day want API named CompileOptions

milseman · 2022-08-11T14:24:54Z

Sources/_StringProcessing/Engine/Processor.swift

@@ -89,6 +89,8 @@ struct Processor {
  // MARK: Metrics, debugging, etc.
  var cycleCount = 0
  var isTracingEnabled: Bool
+  let shouldMeasureMetrics: Bool


While well-predicted, do we see any impact from checking this bool inside the processor? Worst case we could make it conditionally compile and the benchmark can enable it.

Ah... the answer is quite a lot. Removing both the tracing and metric conditions speeds things up by 10-20% across the board. I'll see if I can figure out the conditional compilation

After looking around it doesn't seem like swiftPM supports the feature of "define this compilation variable only when being built as a dependency for target X". The only conditions it can enable/disable compilation variables on are platforms and debug/release. Trying to add multiple targets with flags enabled/disabled for _StringProcessing ends up with "overlapping source code"

https://forums.swift.org/t/swiftpm-and-conditional-compilation/36874
https://forums.swift.org/t/compilation-conditions-and-swift-packages/34627/4

So for now in order to use metrics/tracing you just have to rebuild with the flag directly swift build -c release -Xswiftc -DPROCESSOR_MEASUREMENTS_ENABLED

milseman

LGTM after making sure to underscore the SPI

- make charts prettier - underscore compileroptions - cleanup debug

rctcwyvrn · 2022-08-15T19:56:53Z

Made the charts nice and pretty <:

Ideally this would be done in Package.swift instead of having to add a flag to swift build but alas it appears that it is not possible - https://forums.swift.org/t/swiftpm-and-conditional-compilation/36874 - https://forums.swift.org/t/compilation-conditions-and-swift-packages/34627/4

rctcwyvrn · 2022-08-15T20:28:55Z

@swift-ci test

rctcwyvrn · 2022-08-15T20:34:47Z

@swift-ci test

rctcwyvrn · 2022-08-15T20:39:01Z

@swift-ci test

rctcwyvrn · 2022-08-15T21:33:51Z

augh I forgot to switch it back to squash... oops

rxwei and others added 9 commits July 16, 2022 00:27

Add option to show benchmark charts

49c1d8c

Add new comparison features

6ca5cfd

- Compare with NS regular expression - Save comparison results - Save and compare estimated compile times

Add loading

74dd8e6

Accidently flipped the order

662870e

Disable compile time comparisons by default

1b761be

Change chart to be a normalized performance graph

e8d273a

A bad one because I have no idea how to use Swift Charts

Add compile time measurement + cleanup

2c12236

Oops

1f6010c

Add enum for comparison type

a0af345

rxwei approved these changes Aug 3, 2022

View reviewed changes

rxwei reviewed Aug 3, 2022

View reviewed changes

Merge branch 'main' into charts-and-compares

aff32e6

milseman reviewed Aug 4, 2022

View reviewed changes

rctcwyvrn added 9 commits August 4, 2022 13:14

Add canImport for CI

aa81037

Add compiler options for tracing and metrics

8cdc76e

Add metrics + update tracing

8101f55

Enable metrics and tracing on the benchmarker

b9f68c8

Fix tracing

4b107a8

Cleanup benchmark protocols

9062855

Add parse time measurement + cleanup benchmarker

b2a49f9

Call parse directly

1de8128

Make comparison generic + Adjust stdev requirement to be a %

205e4d3

Cleanup

82f71dd

milseman reviewed Aug 11, 2022

View reviewed changes

milseman approved these changes Aug 11, 2022

View reviewed changes

Cleanup

3d0789f

- make charts prettier - underscore compileroptions - cleanup debug

Fix spacing

dbcb7fd

Fix ns suffix mismatch

c2b672f

rctcwyvrn merged commit 68f8845 into swiftlang:main Aug 15, 2022

More benchmarker features #595

More benchmarker features #595

Uh oh!

Conversation

rctcwyvrn commented Aug 3, 2022

Uh oh!

rxwei left a comment

Choose a reason for hiding this comment

Uh oh!

rxwei Aug 3, 2022

Choose a reason for hiding this comment

Uh oh!

rctcwyvrn commented Aug 3, 2022

Uh oh!

rctcwyvrn commented Aug 3, 2022

Uh oh!

rxwei commented Aug 4, 2022

Uh oh!

milseman Aug 4, 2022

Choose a reason for hiding this comment

Uh oh!

milseman Aug 4, 2022

Choose a reason for hiding this comment

Uh oh!

rctcwyvrn commented Aug 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rctcwyvrn commented Aug 10, 2022

Uh oh!

rctcwyvrn commented Aug 10, 2022

Uh oh!

milseman Aug 10, 2022

Choose a reason for hiding this comment

Uh oh!

milseman Aug 10, 2022

Choose a reason for hiding this comment

Uh oh!

milseman Aug 11, 2022

Choose a reason for hiding this comment

Uh oh!

rctcwyvrn Aug 15, 2022

Choose a reason for hiding this comment

Uh oh!

rctcwyvrn Aug 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

milseman left a comment

Choose a reason for hiding this comment

Uh oh!

rctcwyvrn commented Aug 15, 2022

Uh oh!

rctcwyvrn commented Aug 15, 2022

Uh oh!

rctcwyvrn commented Aug 15, 2022

Uh oh!

rctcwyvrn commented Aug 15, 2022

Uh oh!

rctcwyvrn commented Aug 15, 2022

Uh oh!

Uh oh!

rctcwyvrn commented Aug 5, 2022 •

edited

Loading

rctcwyvrn Aug 15, 2022 •

edited

Loading