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

Rewrite LOAD_GLOBAL _Unbound to LOAD_IMMEDIATE in builtins module #420

Draft
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

tekknolagi
Copy link
Owner

@tekknolagi tekknolagi commented Dec 7, 2022

This should save some caches and be faster. Ideally we would do this in
all Skybison-controlled modules but I don't think we have a good signal
for that yet.

Partially addresses #395

@tekknolagi tekknolagi force-pushed the mb-unbound-immediate branch 2 times, most recently from c33475e to 1d219e9 Compare December 7, 2022 05:49
@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@tekknolagi
Copy link
Owner Author

tekknolagi commented Dec 7, 2022

Hm. Looks like this is a big regression in the rewrite process since nqueens makes so many functions on the fly.

Potential ways around this:

  • Inline generator comprehensions
  • Hoist generator comprehensions to top-level functions
  • Move rewriting to code objects (not great; they're supposed to be immutable)
  • Do this rewrite in the (Python) compiler

@tekknolagi tekknolagi force-pushed the mb-unbound-immediate branch 2 times, most recently from 58ff7ca to df60f02 Compare March 22, 2023 22:57
@github-actions

This comment was marked as outdated.

@github-actions

This comment was marked as outdated.

@tekknolagi tekknolagi force-pushed the mb-unbound-immediate branch from ea54efe to 7fdf49f Compare March 23, 2023 13:30
@github-actions

This comment was marked as outdated.

@tekknolagi
Copy link
Owner Author

Bleh. Nqueens still has a 0.4% regression because rewriting takes longer.

This should save some caches and be faster. Ideally we would do this in
all Skybison-controlled modules but I don't think we have a good signal
for that yet.

Partially addresses #395; we still cannot use it as marker in
dictionaries because it's still in the globals dict.
@tekknolagi tekknolagi force-pushed the mb-unbound-immediate branch from 7fdf49f to 7f11ea6 Compare May 14, 2023 02:36
@github-actions
Copy link

{
  "django_minimal_requests": {
    "benchmark": "django_minimal_requests",
    "cg_instructions before": 720237,
    "cg_instructions now": 720349,
    "cg_instructions ∆": "0.0%",
    "interpreter_args": [],
    "interpreter_name": "pyro",
    "version before": "5215bf66fdf658b5a1758e9e18955ad42310e570",
    "version now": "f10292b021f4e5ff072b466d771f0f0a378cbc15"
  }
}

@github-actions
Copy link

Summary

Metric Average Best Worst Notes
cg_instructions 0.1% bench_base64 -0.1% nqueens 0.6% typically < 0.2% noise
Benchmark details

Base vs. New

benchmark cg_instructions
2to3 0.1%
bench_base64 -0.1%
bench_compile 0.1%
bench_pickle 0.0%
deltablue 0.1%
fannkuch -0.0%
go 0.1%
loadproperty 0.3%
nbody 0.0%
nqueens 0.6%
pyflate -0.0%
pystone 0.1%
richards 0.1%

CPython vs New

benchmark cg_instructions
2to3 -8.6%
bench_base64 -38.8%
bench_compile 1117.1%
bench_pickle -19.4%
deltablue -64.5%
fannkuch -1.3%
go -64.0%
loadproperty -75.1%
nbody 20.7%
nqueens 42.8%
pyflate -30.6%
pystone -71.8%
richards -80.0%

Base

benchmark cg_instructions
2to3 2,342,001,511
bench_base64 2,954,425,156
bench_compile 3,006,175,526
bench_pickle 3,341,162,384
deltablue 1,459,393,975
fannkuch 5,521,747,288
go 1,893,969,866
loadproperty 434,780,405
nbody 9,406,459,048
nqueens 3,141,766,729
pyflate 10,038,792,535
pystone 1,122,683,016
richards 964,856,353

New

benchmark cg_instructions
2to3 2,343,890,737
bench_base64 2,951,823,816
bench_compile 3,009,519,754
bench_pickle 3,341,455,395
deltablue 1,460,414,307
fannkuch 5,519,137,067
go 1,895,104,993
loadproperty 435,916,734
nbody 9,407,598,312
nqueens 3,160,980,700
pyflate 10,035,975,823
pystone 1,123,820,711
richards 965,996,283

CPython

benchmark cg_instructions
2to3 2,563,610,701
bench_base64 4,820,248,126
bench_compile 247,270,261
bench_pickle 4,143,805,223
deltablue 4,109,440,109
fannkuch 5,594,149,369
go 5,264,709,090
loadproperty 1,747,774,500
nbody 7,793,864,282
nqueens 2,213,176,123
pyflate 14,468,046,983
pystone 3,981,585,959
richards 4,821,559,159

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.

1 participant