changes/simplifications/bugfixes to support v1.12 #147
Conversation
This simplifies the generated function to avoid the runtime edge against the method (instead passing that as an explicit argument to the generator), and then fixes the calls into the Compiler module to correctly forward the world information back through the generator.
| # get typed code | ||
| codeinfos = Core.eval(mod, code_typed(inferfn, Tuple; optimize=false)) | ||
| m = only(methods(inferfn, Tuple)) | ||
| codeinfo = only(code_typed(inferfn, Tuple; optimize=false)) |
There was a problem hiding this comment.
Something to consider after this PR: this pre-work analysis here is invalid now that binding replacement is possible. Does it provide any useful result, or can this just be deleted (along with all of the associated complexity it causes)?
There was a problem hiding this comment.
Supposedly much of this complexity was necessary for the generated coroutine to be fast. Without all of this, the generated structure representing the finite state machine did not have its fields concretely typed, leading to a ton of boxing and allocations.
Or maybe I am completely misunderstanding. Regrettably, currently I am the only "maintainer", but I myself am not particularly knowledgeable of the compiler internals that this package depends on. Maybe more of a "care taker" than a "maintainer".
There was a problem hiding this comment.
Gotcha. Yeah, I did some git splunking and it appears that this should have been removed in #76, but apparently got left behind as detritus of the original design limitations. I'll take that as license to clean this up more.
I am interested in helping maintain this, particularly as an example project of how to use the compiler internals well. I have some ideas in mind for more drastic improvements.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #147 +/- ##
==========================================
+ Coverage 79.55% 80.86% +1.31%
==========================================
Files 6 6
Lines 680 690 +10
==========================================
+ Hits 541 558 +17
+ Misses 139 132 -7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@vtjnash , it seems most dependencies of ResumableFunctions were still working fine on 1.12. Could you let me know how you were made aware of these problems? That would help me in making sure the tests are actually not missing anything. |
|
It might be v1.13 then, since I'm not actually good at remembering which version we are on. It has been failing in PkgEval for some time: |
That would be greatly beneficial in many ways. (1) I personally am searching for good onboarding materials to learn more about the compiler. (2) Having one of the few deep julia wizards vet the quality of this package would make me much more comfortable basing production code on top of ResumableFunctions. I will go ahead and add you the package owners so that you can more freely make changes to this repository. While I very much like the idea of code reviews, this package currently does not have deeply knowledgeable maintainers, and you have pretty significant credentials, so do not hesitate to merge your PRs without waiting for review, as long as the downstream tests (especially ConcurrentSim.jl) pass. On the other hand, please keep updating the changelog as you make changes -- otherwise I would truly be out of my depth when you move on to your next pet project. FYI, if you find this useful, I consent to bumping the compat to julia 1.12. And mildly-breaking releases are not out of the question -- the package does not have that many dependents and I can commit to submitting PRs to them if changes happen. Lastly, the benchmarks.jl script is reasonably representative of simple tasks that have been difficult to keep properly inferred / fast as the library has evolved. |
|
Thanks! That is good to know. I hope to keep things compatible (with optional flags to opt in for new behaviors and fixes), but perhaps that means we'll eventually drop old implementations if there aren't any users who end up needing them |
3 participants
This simplifies the generated function to avoid the runtime edge against
the method (instead passing that as an explicit argument to the
generator), and then fixes the calls into the Compiler module to
correctly forward the world information back through the generator.