Skip to content

Fix debugger scope handling during early returns #1528

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 15, 2024
Merged

Conversation

swernli
Copy link
Collaborator

@swernli swernli commented May 15, 2024

Changes introduced in #1442 to enable debugger tracking of variable scopes caused a bug if a program used callables with early returns, namely those returns would not pop any added scopes and later execution would read/write the wrong variable states. This fixes the issue by introducing a debug-only return node that triggers a frame leave handling in the environment. This needs to be debug only because it should not be used during partial evaluation.

Changes introduced in #1442 to enable debugger tracking of variable scopes caused a bug if a program used callables with early returns, namely those returns would not pop an added scopes and later execution would read/write the wrong variable states. This fixes  the issue by introducing a debug-only return node that triggers a frame leave handling in the environment. This needs to be debug only because it should not be used during partial evaluation.
Copy link

Benchmark for 0afac77

Click to view benchmark
Test Base PR %
Array append evaluation 344.7±6.37µs 331.0±10.69µs -3.97%
Array literal evaluation 200.0±4.28µs 171.4±2.09µs -14.30%
Array update evaluation 434.9±2.47µs 412.6±1.53µs -5.13%
Core + Standard library compilation 20.5±1.17ms 20.4±1.30ms -0.49%
Deutsch-Jozsa evaluation 5.1±0.04ms 5.2±0.15ms +1.96%
Large file parity evaluation 33.9±0.12ms 33.7±0.54ms -0.59%
Large input file compilation 14.4±0.46ms 13.7±0.36ms -4.86%
Large input file compilation (interpreter) 51.1±2.08ms 51.8±1.81ms +1.37%
Large nested iteration 33.8±0.17ms 33.1±0.24ms -2.07%
Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample 1616.9±159.20µs 1638.8±211.67µs +1.35%
Perform Runtime Capabilities Analysis (RCA) on large file sample 8.2±0.17ms 8.4±0.17ms +2.44%
Perform Runtime Capabilities Analysis (RCA) on teleport sample 1495.7±211.15µs 1487.0±170.75µs -0.58%
Perform Runtime Capabilities Analysis (RCA) on the core and std libraries 29.0±0.28ms 28.9±0.17ms -0.34%
Teleport evaluation 89.1±5.51µs 90.1±3.51µs +1.12%

@swernli
Copy link
Collaborator Author

swernli commented May 15, 2024

Regarding testing, I'm thinking it would make sense to add some targeted unit tests to the debugger tests in this PR and then add execution of all the samples to integration tests. We'll have to decide how to handle the resource estimation samples so the test execution is not too long from trying to simulate a scalable algorithm, but I don't want to delay this PR while we work that out.

@swernli swernli marked this pull request as ready for review May 15, 2024 20:54
@swernli swernli requested review from idavis and minestarks as code owners May 15, 2024 20:54
Copy link

Benchmark for c4cad6b

Click to view benchmark
Test Base PR %
Array append evaluation 334.6±2.65µs 326.5±1.73µs -2.42%
Array literal evaluation 186.3±0.78µs 176.9±1.15µs -5.05%
Array update evaluation 414.1±2.29µs 408.5±3.75µs -1.35%
Core + Standard library compilation 17.3±0.33ms 17.0±0.08ms -1.73%
Deutsch-Jozsa evaluation 5.1±0.05ms 5.0±0.04ms -1.96%
Large file parity evaluation 33.9±0.13ms 34.1±0.19ms +0.59%
Large input file compilation 12.2±0.28ms 12.2±0.36ms 0.00%
Large input file compilation (interpreter) 46.7±2.06ms 43.8±0.82ms -6.21%
Large nested iteration 32.6±0.17ms 32.5±0.23ms -0.31%
Perform Runtime Capabilities Analysis (RCA) on Deutsch-Jozsa sample 1567.2±43.55µs 1550.8±28.09µs -1.05%
Perform Runtime Capabilities Analysis (RCA) on large file sample 7.8±0.06ms 7.7±0.07ms -1.28%
Perform Runtime Capabilities Analysis (RCA) on teleport sample 1432.9±46.43µs 1416.5±28.28µs -1.14%
Perform Runtime Capabilities Analysis (RCA) on the core and std libraries 27.4±0.59ms 27.0±0.15ms -1.46%
Teleport evaluation 87.5±3.76µs 87.0±3.80µs -0.57%

@swernli swernli added this pull request to the merge queue May 15, 2024
Merged via the queue into main with commit eb4b81b May 15, 2024
16 checks passed
@swernli swernli deleted the swernli/debug-scope-fix branch May 15, 2024 21:55
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.

2 participants