Add AsyncSyntaxRewriter

[roopekv]

This is a prerequisite for implementing async overloads of assertMacroExpansion and the various expansion functions discussed with @ahoppen at #2803 (comment). Specifically, AsyncSyntaxRewriter is required to be able to implement an asynchronous version of MacroApplication:

swift-syntax/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift

Line 659 in 16dc4f8

private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {

AsyncSyntaxRewriter has the same visibility as SyntaxRewriter, but in terms of MacroApplication, the visibility of AsyncSyntaxRewriter could be changed to internal if its declaration was moved from SwiftSyntax to SwiftSyntaxMacroExpansion module.

Changes made to SyntaxRewriter code generation also make it easy to generate throws and async throws versions of SyntaxRewriter if they are at some point desired.

Another potential name for AsyncSyntaxRewriter could be SyntaxAsyncRewriter, similar to SyntaxAnyVisitor, but I am not sure which one is better.

[ahoppen]

Could you check how the performance of AsyncSyntaxRewriter compares to that of SyntaxRewriter? A pretty easy test would be to use VisistorPerformanceTests.testEmptyRewriterPerformance. I’m curious whether adding async to all the visit functions has any performance impact.

[roopekv]

Results

testEmptyRewriterPerformance

1 time:    ~0.461 s
100 times: ~46.26 s

testEmptyAsyncRewriterPerformance

1 time:    ~0.475 s
100 times: ~47.54 s

Code

testEmptyRewriterPerformance

swift-syntax/Tests/PerformanceTest/VisitorPerformanceTests.swift

Lines 40 to 51 in fe1a312

	func testEmptyRewriterPerformance() throws {
	try XCTSkipIf(longTestsDisabled)
	class EmptyRewriter: SyntaxRewriter {}
	let source = try String(contentsOf: inputFile, encoding: .utf8)
	let parsed = Parser.parse(source: source)
	let emptyRewriter = EmptyRewriter(viewMode: .sourceAccurate)
	try measureInstructions {
	_ = emptyRewriter.rewrite(parsed)
	}
	}

swift-syntax/Tests/PerformanceTest/InstructionsCountAssertion.swift

Lines 26 to 74 in fe1a312

	func measureInstructions(
	_ baselineName: StaticString = #function,
	block: () -> Void,
	file: StaticString = #filePath,
	line: UInt = #line
	) throws {
	let startInstructions = getInstructionsExecuted()
	block()
	let endInstructions = getInstructionsExecuted()
	let numberOfInstructions = endInstructions - startInstructions
	let strippedBaselineName = "\(baselineName)".replacingOccurrences(of: "()", with: "")
	// Performance testing is only supported on macOS.
	// On all other platforms `getInstructionsExecuted` returns 0.
	#if os(macOS)
	// If the is no data, we just continue the test
	guard let data = try? Data(contentsOf: baselineURL) else {
	return
	}
	let jsonDecoder = JSONDecoder()
	let baselineMap = try jsonDecoder.decode([String: UInt64].self, from: data)
	guard let baseline = baselineMap[strippedBaselineName] else {
	XCTFail(
	"""
	Missing baseline for \(strippedBaselineName)
	with number of instructions '\(numberOfInstructions)'
	""",
	file: file,
	line: line
	)
	return
	}
	let relativeDeviation = Double(numberOfInstructions) / Double(baseline) - 1
	let allowedDeviation = 0.01
	XCTAssertTrue(
	(-allowedDeviation..<allowedDeviation).contains(relativeDeviation),
	"""
	Number of instructions '\(numberOfInstructions)' deviated from baseline by \(String(format: "%.4f", relativeDeviation * 100))%.
	The maximum allowed deviation for '\(strippedBaselineName)' is \(allowedDeviation * 100)% in either direction.
	""",
	file: file,
	line: line
	)
	#endif
	}

testEmptyAsyncRewriterPerformance

func testEmptyAsyncRewriterPerformance() async throws {
  try XCTSkipIf(longTestsDisabled)
  class EmptyAsyncRewriter: AsyncSyntaxRewriter {}

  let source = try String(contentsOf: inputFile, encoding: .utf8)
  let parsed = Parser.parse(source: source)
  let emptyRewriter = EmptyAsyncRewriter(viewMode: .sourceAccurate)

  try await measureInstructions {
    _ = await emptyRewriter.rewrite(parsed)
  }
}

func measureInstructions(
  _ baselineName: StaticString = #function,
  block: () async -> Void,
  file: StaticString = #filePath,
  line: UInt = #line
) async throws {
  let startInstructions = getInstructionsExecuted()
  await block()
  let endInstructions = getInstructionsExecuted()
  let numberOfInstructions = endInstructions - startInstructions
  let strippedBaselineName = "\(baselineName)".replacingOccurrences(of: "()", with: "")

  // Performance testing is only supported on macOS.
  // On all other platforms `getInstructionsExecuted` returns 0.
  #if os(macOS)
  // If the is no data, we just continue the test
  guard let data = try? Data(contentsOf: baselineURL) else {
    return
  }

  let jsonDecoder = JSONDecoder()
  let baselineMap = try jsonDecoder.decode([String: UInt64].self, from: data)

  guard let baseline = baselineMap[strippedBaselineName] else {
    XCTFail(
      """
      Missing baseline for \(strippedBaselineName)
      with number of instructions '\(numberOfInstructions)'
      """,
      file: file,
      line: line
    )
    return
  }

  let relativeDeviation = Double(numberOfInstructions) / Double(baseline) - 1
  let allowedDeviation = 0.01

  XCTAssertTrue(
    (-allowedDeviation..<allowedDeviation).contains(relativeDeviation),
    """
    Number of instructions '\(numberOfInstructions)' deviated from baseline by \(String(format: "%.4f", relativeDeviation * 100))%.
    The maximum allowed deviation for '\(strippedBaselineName)' is \(allowedDeviation * 100)% in either direction.
    """,
    file: file,
    line: line
  )
  #endif
}

Log

testEmptyRewriterPerformance

Test Suite 'Selected tests' started at 2024-08-26 19:26:08.230.
Test Suite 'PerformanceTest.xctest' started at 2024-08-26 19:26:08.230.
Test Suite 'VisitorPerformanceTests' started at 2024-08-26 19:26:08.230.
Test Case '-[PerformanceTest.VisitorPerformanceTests testEmptyRewriterPerformance]' started (Iteration 1 of 100).
Test Case '-[PerformanceTest.VisitorPerformanceTests testEmptyRewriterPerformance]' passed (0.483 seconds).
Test Case '-[PerformanceTest.VisitorPerformanceTests testEmptyRewriterPerformance]' started (Iteration 2 of 100).
Test Case '-[PerformanceTest.VisitorPerformanceTests testEmptyRewriterPerformance]' passed (0.468 seconds).
Test Case '-[PerformanceTest.VisitorPerformanceTests testEmptyRewriterPerformance]' started (Iteration 3 of 100).
Test Case '-[PerformanceTest.VisitorPerformanceTests testEmptyRewriterPerformance]' passed (0.463 seconds).
...
Test Case '-[PerformanceTest.VisitorPerformanceTests testEmptyRewriterPerformance]' started (Iteration 100 of 100).
Test Case '-[PerformanceTest.VisitorPerformanceTests testEmptyRewriterPerformance]' passed (0.461 seconds).
Test Suite 'VisitorPerformanceTests' passed at 2024-08-26 19:26:54.485.
	 Executed 100 tests, with 0 failures (0 unexpected) in 46.238 (46.255) seconds
Test Suite 'PerformanceTest.xctest' passed at 2024-08-26 19:26:54.485.
	 Executed 100 tests, with 0 failures (0 unexpected) in 46.238 (46.255) seconds
Test Suite 'Selected tests' passed at 2024-08-26 19:26:54.485.
	 Executed 100 tests, with 0 failures (0 unexpected) in 46.238 (46.255) seconds
Program ended with exit code: 0

testEmptyAsyncRewriterPerformance

Test Suite 'Selected tests' started at 2024-08-26 19:24:18.137.
Test Suite 'PerformanceTest.xctest' started at 2024-08-26 19:24:18.137.
Test Suite 'VisitorPerformanceTests' started at 2024-08-26 19:24:18.137.
Test Case '-[PerformanceTest.VisitorPerformanceTests testEmptyAsyncRewriterPerformance]' started (Iteration 1 of 100).
Test Case '-[PerformanceTest.VisitorPerformanceTests testEmptyAsyncRewriterPerformance]' passed (0.494 seconds).
Test Case '-[PerformanceTest.VisitorPerformanceTests testEmptyAsyncRewriterPerformance]' started (Iteration 2 of 100).
Test Case '-[PerformanceTest.VisitorPerformanceTests testEmptyAsyncRewriterPerformance]' passed (0.478 seconds).
Test Case '-[PerformanceTest.VisitorPerformanceTests testEmptyAsyncRewriterPerformance]' started (Iteration 3 of 100).
Test Case '-[PerformanceTest.VisitorPerformanceTests testEmptyAsyncRewriterPerformance]' passed (0.476 seconds).
...
Test Case '-[PerformanceTest.VisitorPerformanceTests testEmptyAsyncRewriterPerformance]' started (Iteration 100 of 100).
Test Case '-[PerformanceTest.VisitorPerformanceTests testEmptyAsyncRewriterPerformance]' passed (0.475 seconds).
Test Suite 'VisitorPerformanceTests' passed at 2024-08-26 19:25:05.681.
	 Executed 100 tests, with 0 failures (0 unexpected) in 47.529 (47.543) seconds
Test Suite 'PerformanceTest.xctest' passed at 2024-08-26 19:25:05.681.
	 Executed 100 tests, with 0 failures (0 unexpected) in 47.529 (47.544) seconds
Test Suite 'Selected tests' passed at 2024-08-26 19:25:05.681.
	 Executed 100 tests, with 0 failures (0 unexpected) in 47.529 (47.544) seconds
Program ended with exit code: 0

[roopekv]

(the results in the above comment were obtained running in debug mode)

Results running the tests in release mode:

testEmptyRewriterPerformance

1 time:     ~0.018 s
100 times:  ~1.810 s
1000 times: ~17.84 s

testEmptyAsyncRewriterPerformance

1 time:     ~0.025 s
100 times:  ~2.477 s
1000 times: ~24.92 s

Both of the rewriters traverse the syntax tree serially with no parallelism, so it makes sense that the async one is slightly slower due to the overhead of doing things asynchronously. However, in cases where the visit functions contain more work the difference should be smaller, as the time spent doing asynchronous operations stays the same while the overall duration increases, and in cases where parallelism can be utilized, the async rewriter has the potential to be faster than the sync one.

[ahoppen]

Oh, wow. 40% performance decrease in the AsyncSyntaxRewriter is quite significant… I will need to think about this and its implementations a bit and will come back to you.

CC @rintaro

[roopekv]

I guess the question is, would it be useful to expose AsyncSyntaxRewriter as public for everyone to use, or should I change it to internal and move it to the SwiftSyntaxMacroExpansion module?

If it's decided that AsyncSyntaxRewriter should not be public, should I combine AsyncSyntaxRewriter and the async overloads of assertMacroExpansion and expansion functions into a single PR and close this one?

[roopekv]

Oh, wow. 40% performance decrease in the AsyncSyntaxRewriter is quite significant… I will need to think about this and its implementations a bit and will come back to you.

Yeah, it's not great... Good thing you brought up the topic of performance. I'll try to come up with a non-recursive solution ditching SyntaxRewriter based MacroApplication where the use of await would be limited to cases where it's required to be able to call expansion or where things can be parallelized.