Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 13, 2025

Expansion analyzers can take excessive time on large files. This adds load tests to catch performance regressions.

Changes

  • test/analyzer-load-test.rkt: Load tests for all four expansion analyzers (identifier-usage, variable-mutability, ignored-result-values, function-expression)
    • Generates ~300 line test program with functions, variables, mutations, and nested expressions
    • Uses expansion-analyze directly with time-apply to measure actual analyzer performance
    • Pre-expands syntax once using source-expand and reuses it for all tests
    • Verifies each analyzer completes within 30 second timeout
    • All tests are in a (module+ test ...) submodule

Each test measures elapsed time and fails with diagnostic message if timeout exceeded:

(test-case "identifier-usage-analyzer load test"
  (define-values (result cpu-time real-time gc-time)
    (time-apply expansion-analyze (list identifier-usage-analyzer expanded-syntax)))
  
  (check-true (< real-time timeout-ms)
              (format "identifier-usage-analyzer took ~a ms, expected < ~a ms"
                      real-time timeout-ms)))
Original prompt

This section details on the original issue you should resolve

<issue_title>Analyzer load tests</issue_title>
<issue_description>Expansion analyzers can easily take much longer than expected on large files. There should be a load test for each analyzer that asserts that the analyzer is capable of analyzing a suitably large file within a reasonable amount of time. This should use the analyzer timeout configuration option mentioned in #733.</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI changed the title [WIP] Add load tests for each analyzer performance Add load tests for expansion analyzers Nov 13, 2025
Copilot AI requested a review from jackfirth November 13, 2025 17:25
Copilot finished work on behalf of jackfirth November 13, 2025 17:25
@jackfirth jackfirth marked this pull request as ready for review November 14, 2025 00:22
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resyntax analyzed 1 file in this pull request and found no issues.

@coveralls
Copy link

Coverage Status

coverage: 93.71% (+0.05%) from 93.661%
when pulling 06f12ac on copilot/add-analyzer-load-tests
into ef1a5e9 on master.

Copy link
Owner

@jackfirth jackfirth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put all of this inside of a (module+ test ...) submodule.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 1ab6849. All code is now inside (module+ test ...).

Comment on lines 89 to 92
(define analysis
(source-analyze large-test-source
#:analyzers (list variable-mutability-analyzer)
#:timeout-ms timeout-ms))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't work. The source-analyze will kill the analyzer after timeout-ms and return an empty source-analysis? value while logging a warning. So the test cases in this file will always pass.

Instead of using source-analyze, call the expansion analyzer directly (with expansion-analyze) by passing it fully-expanded syntax created using source-expand in a new base namespace. Use time-apply to measure how long the analyzer takes to process the expanded syntax.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 1ab6849. Now using expansion-analyze directly with time-apply to measure actual analyzer performance. The syntax is expanded once using source-expand and reused for all tests.

Comment on lines 142 to 165
(test-case "all analyzers together load test"
;; This test ensures all analyzers can work together on a large file
;; within the timeout period.
(define all-analyzers
(list identifier-usage-analyzer
variable-mutability-analyzer
ignored-result-values-analyzer
function-expression-analyzer))

(define start-time (current-inexact-milliseconds))
(define analysis
(source-analyze large-test-source
#:analyzers all-analyzers
#:timeout-ms timeout-ms))
(define elapsed-ms (- (current-inexact-milliseconds) start-time))

;; Verify the analysis completed successfully
(check-true (source-code-analysis? analysis))

;; Verify it completed within the timeout (with some margin)
(check-true (< elapsed-ms timeout-ms)
(format "all analyzers together took ~a ms, expected < ~a ms"
elapsed-ms
timeout-ms)))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't bother with this load test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in 1ab6849.

- Wrap everything in (module+ test ...) submodule
- Use expansion-analyze directly instead of source-analyze
- Use time-apply to measure timing
- Expand syntax once and reuse for all tests
- Remove "all analyzers together" test

Co-authored-by: jackfirth <[email protected]>
Copilot AI requested a review from jackfirth November 14, 2025 08:01
Copilot finished work on behalf of jackfirth November 14, 2025 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Analyzer load tests

3 participants