Skip to content
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

ref: split ast parsing / class exec loops #7248

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

jordanrfrazier
Copy link
Collaborator

@jordanrfrazier jordanrfrazier commented Mar 24, 2025

Uses Artemis by TurinTech to find optimizations and quality improvements. These changes were selected as part of a Sourcekon profiling, after which Artemis Intelligence was used to suggest multiple versions of improvements based on iterative scoring feedback.

Specifically, these changes

  • Utilize loop fission to improve readability, minimize operations in instruction cache to potentially improve cache locality, and pre-group definitions for a single compilation and execution.
  • Improve error handling

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Mar 24, 2025
Copy link
Contributor

@ogabrielluiz ogabrielluiz left a comment

Choose a reason for hiding this comment

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

This looks good, @jordanrfrazier !

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Mar 24, 2025
@jordanrfrazier
Copy link
Collaborator Author

@hughEvansOne ptal. Great work on Artemis. Looking forward to building better benchmarks so we can quantitatively see the improvements your team has on this project!

Copy link

codspeed-hq bot commented Mar 24, 2025

CodSpeed Performance Report

Merging #7248 will improve performances by 57.46%

Comparing various-qol-code-improvements (b05dfea) with main (0eeb142)

Summary

⚡ 4 improvements
✅ 15 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
test_build_flow_invalid_job_id 9 ms 7.9 ms +13.97%
test_cancel_build_unexpected_error 899.9 ms 756.2 ms +19%
test_cancel_build_with_cancelled_error 416.5 ms 264.5 ms +57.46%
test_cancel_nonexistent_build 12 ms 8.2 ms +46.37%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants