MatthieuDartiailh / bytecode

Python module to modify bytecode
https://bytecode.readthedocs.io/
MIT License
302 stars 38 forks source link

ci: improve code round-tripping #143

Closed P403n1x87 closed 2 weeks ago

P403n1x87 commented 7 months ago

We improve the logic for round-trip re-compilation of code objects by listening for module code objects before they are executed. This way we can thoroughly recurse over nested code object and attain total coverage for better confidence.

MatthieuDartiailh commented 7 months ago

And sadly it breaks. It will likely take me some time before I can look into this though.

P403n1x87 commented 7 months ago

And sadly it breaks. It will likely take me some time before I can look into this though.

No worries. I haven't come across any issues so these might be a few corner cases, and only on 3.11 https://github.com/MatthieuDartiailh/bytecode/actions/runs/8344140634/job/22835903388?pr=143

MatthieuDartiailh commented 7 months ago

It is nice to see that 3.12 is fine !

P403n1x87 commented 7 months ago

Maybe something to look into is why it seems to take about twice as long on 3.12 to recompile about the same number of code objects

Recompiled 11901 code objects in 0:00:48.426503
MatthieuDartiailh commented 7 months ago

Compared to what ? 3.10 ?

To me that is not that surprising due to the introduction of exception tables in 3.11.

P403n1x87 commented 7 months ago

3.11 took 36s (I think we can assume it re-compiled all code objects without errors). 3.10 and earier are all in the ballpark of 25s. It makes sense for 3.11 and later versions to take more time because of the extra data structures, but the exception table is generally much smaller than the bytecode itself, so I don't know if that alone can really explain a 100% run time increase. I'll see if I can get some CPU profiling data

MatthieuDartiailh commented 2 months ago

@P403n1x87 did you ever manage to generate the profiling data ?

codecov-commenter commented 4 weeks ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 95.79%. Comparing base (63d32b5) to head (f4727c9).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #143 +/- ## ======================================= Coverage 95.79% 95.79% ======================================= Files 7 7 Lines 2044 2044 Branches 464 464 ======================================= Hits 1958 1958 Misses 52 52 Partials 34 34 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

P403n1x87 commented 3 weeks ago

@MatthieuDartiailh the tests on 3.13 pass without the re-compilation. It looks like the wrong arguments are being resolved. I have added

        print("availabe_services", available_services)

and these are the results

With recomp

unit/test_session.py availabe_services Session(region_name=<Mock name='mock.get_config_variable()' id='4875256224'>)
Favailabe_services Session(region_name=<Mock name='mock.get_config_variable()' id='4875263280'>)

Without recomp

unit/test_session.py availabe_services ['good-resource']
.availabe_services ['good-resource']
P403n1x87 commented 3 weeks ago

@MatthieuDartiailh I have the feeling that the CFG needs a similar fix to #145

    assert te.entry is self._current_try_begin

WDYT?

MatthieuDartiailh commented 2 weeks ago

Superseded by #149