Run the compiler stages in a browser, and build the constants list ourselves - #80
Merged
Conversation
…rselves Closes #77. Closes #78. The WebAssembly build's compiler_codegen returns metadata with no consts key, so pyxray.compiler.stages raised KeyError on the line that passed it to optimize_cfg, and the middle compiler stage did not run in a browser at all. It builds that list from the instruction sequence now, one placeholder per slot, and records in Stages.constants_known whether the values were real. The optimizer without the values is not the optimizer doing nothing. It cannot fold 6 * 7 into 42, and on code like while 0: it makes a different decision than it would with them, so anything that reports what the optimizer did has to check the flag first. what_the_optimizer_did says so under the two columns, the pipeline widget marks that pane and explains it, and T05 prints compiler.constants_available() next to compiler.available() and says what a False there means. The placeholder is a class rather than None on purpose. The optimizer rewrites a LOAD_CONST of None into the shorter LOAD_COMMON_CONSTANT, so padding with None would show a browser reader an optimization that never happened where their source has a 6. The length is the safety part. optimize_cfg reads that list by index, and a list shorter than the largest index reads past the end of memory in the WebAssembly build: no exception, the runtime does not come back, and in a notebook the reader loses their work. So the list is built inside pyxray every time and taken from nobody, which is what closes #78, since the pipeline widget reaches the optimizer only through that function. A parametrized test drives real sources through the widget and checks the length of what it passed. wasmprobe changes too. The optimize_cfg check is no longer how our code calls it, so it drops from tier0 to info and loses its accepted excuse, and optimize_cfg_direct now builds its list the way pyxray does. No check carries an accepted gap any more, so that test builds one rather than asserting the list is not empty.
18 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #77. Closes #78.
Both issues came out of the Pyodide measurement in #76, and they are the same bug seen from two sides, so they are fixed together.
The bug
compiler_codegencollects the constants it comes across and hands them back in its metadata. The WebAssembly build does not: the dictionary hasargcount,kwonlyargcountandposonlyargcountand nothing else.pyxray.compiler.stagespassedmetadata["consts"]straight intooptimize_cfg, so that line raisedKeyErrorin a browser and the middle compiler stage did not run there at all. T05 and the pipeline widget both go through that function, and both are meant to run in a tab with nothing installed.The fix
stagesbuilds the constants list itself now, from the instruction sequence it is about to pass in, and records onStages.constants_knownwhether the values were the real ones. The optimizer runs everywhere. On a build that hands the values over, nothing changes at all.The part that needs saying out loud
The optimizer without the values is not the optimizer doing nothing. It cannot fold
6 * 7into42, and on something likewhile 0:it makes a different decision than it would with them, so the right hand column in a browser is a real answer to a question the reader did not ask. Three places say so now:what_the_optimizer_didadds a line under the two columns, the pipeline widget marks that pane and explains it in a note, and T05 printscompiler.constants_available()next tocompiler.available()and has a paragraph about what a False there means. The last pane and the last cell are the finished code object, which comes from the ordinarycompile()and is the real answer on every build.The placeholder is a small class rather than
None, which matters more than it looks. The optimizer rewrites aLOAD_CONSTof aNoneinto the shorterLOAD_COMMON_CONSTANTform, so padding withNonewould show a browser readerLOAD_COMMON_CONSTANTwhere their source has a 6, which reads as an optimization and is not one. A placeholder it cannot recognise leavesLOAD_CONST 0sitting there, which is honest.The safety half, which is #78
optimize_cfgreads that list by index. Hand it one shorter than the largest index in the sequence and a native interpreter raises a tidyValueError, while the WebAssembly build reads past the end of its memory and does not come back. There is no exception, so there is nothing to catch, and in a notebook it takes the kernel and whatever the reader had done. That is measured, inprobes/pyodide/pyodide.jsonunderoptimize_cfg_short_consts.So the list is built inside
pyxray.compilerevery time and taken from nobody. The pipeline widget, which is the one place a reader types their own source and therefore the easiest way to reach this, has no other route to the optimizer. There is a parametrized test that drives seven real sources through the widget's own path, watches the call, and checks the list was long enough, because the property that has to hold is the length rather than the absence of a crash on an interpreter where a short list would only raise. The rule is written down at the top ofpipeline.pyso the next person to add a parameter there knows why they should not.wasmprobe
The
optimize_cfgcheck was described as "the way pyxray calls it", and pyxray does not call it that way any more. It drops fromtier0toinfoand loses itsacceptedexcuse, and its cost line now says what it used to cost and why it is still worth asking every run.optimize_cfg_directtakes over the tier 0 wording and builds its list the waypyxraydoes, offdis.hasconstrather than a hard codedLOAD_CONST. The recordings and the report are rebuilt.No check carries an accepted gap now, which is the good outcome and not a reason to delete the mechanism, so that test builds a check with one rather than asserting the real list is not empty.
probes/pyodide/decision.mdand the browser section ofCONTRIBUTING.mdare updated to match.Checks
just checkis green: lint clean, 1428 passed and 3 skipped, 535 citations, 12 lessons and 12 notebooks up to date and running, the probe still reporting 15 checks with nothing blocking, and 5 animations.