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

py: don't add traceback info for finally or re-raised exceptions #5032

Closed

Conversation

dpgeorge
Copy link
Member

This is a fix for #2928. It makes sure that traceback info is not added to exceptions that 1) propagate through a finally block; 2) are re-raised with raise (no args).

Eg, before this patch (see #2928 for the test script):

Traceback (most recent call last):
  File "<stdin>", line 19, in <module> (finally)
  File "<stdin>", line 17, in <module> (re-raise)
  File "<stdin>", line 17, in <module> (re-raise)
  File "<stdin>", line 14, in <module>
  File "<stdin>", line 2, in some_fn
  File "<stdin>", line 5, in foo
  File "<stdin>", line 8, in bar
RuntimeError: something

and after this patch:

Traceback (most recent call last):
  File "<stdin>", line 14, in <module>
  File "<stdin>", line 2, in some_fn
  File "<stdin>", line 5, in foo
  File "<stdin>", line 8, in bar
RuntimeError: something

(which matches CPython output).

I think this is a good fix, it will improve error messages (not as confusing), and make finally's slightly more efficient (in time and RAM) by not bothering to add the traceback.

The downside is that it increases code size a bit:

   bare-arm:   +28 +0.042% 
minimal x86:   +16 +0.010% 
   unix x64:   +32 +0.006% 
unix nanbox:   +96 +0.022% 
      stm32:   +20 +0.005% PYBV10
     cc3200:   +32 +0.017% 
    esp8266:   +56 +0.009% 
      esp32:   +24 +0.002% GENERIC
        nrf:   +16 +0.011% pca10040
       samd:   +24 +0.024% ADAFRUIT_ITSYBITSY_M4_EXPRESS

Also, it may reduce VM performance (when raising exception) by a tiny amount due to the extra checks, but I didn't yet measure that.

@dpgeorge
Copy link
Member Author

I did some benchmark tests, on unix:

diff of scores (higher is better)
N=5000 M=1000             unix-base -> unix-fixtb         diff      diff% (error%)
bm_chaos.py                14606.64 ->   14773.99 :    +167.35 =  +1.146% (+/-8.11%)
bm_fannkuch.py                 6.90 ->       7.28 :      +0.38 =  +5.507% (+/-2.50%)
bm_float.py               291231.47 ->  284266.40 :   -6965.07 =  -2.392% (+/-5.16%)
bm_hexiom.py                 683.99 ->     670.85 :     -13.14 =  -1.921% (+/-2.68%)
bm_nqueens.py             299036.40 ->  296267.90 :   -2768.50 =  -0.926% (+/-2.87%)
bm_pidigits.py              2084.89 ->    2077.98 :      -6.91 =  -0.331% (+/-2.08%)
misc_aes.py                16400.92 ->   16209.37 :    -191.55 =  -1.168% (+/-3.67%)
misc_mandel.py            131935.54 ->  129426.66 :   -2508.88 =  -1.902% (+/-7.79%)
misc_pystone.py            88439.61 ->   88937.22 :    +497.61 =  +0.563% (+/-5.05%)
misc_raytrace.py           24725.26 ->   24950.44 :    +225.18 =  +0.911% (+/-5.26%)
viper_call0.py             21125.29 ->   22040.51 :    +915.22 =  +4.332% (+/-14.23%)
viper_call1a.py            23025.42 ->   21819.45 :   -1205.97 =  -5.238% (+/-11.35%)
viper_call1b.py            18390.95 ->   17893.11 :    -497.84 =  -2.707% (+/-10.37%)
viper_call1c.py            17014.19 ->   17559.29 :    +545.10 =  +3.204% (+/-14.35%)
viper_call2a.py            20953.98 ->   20134.57 :    -819.41 =  -3.911% (+/-18.32%)
viper_call2b.py            15586.11 ->   16784.60 :   +1198.49 =  +7.689% (+/-11.98%)

There is no difference within the error margin.

On PYBv1.0:

diff of scores (higher is better)
N=100 M=100                pyb-base ->  pyb-fixtb         diff      diff% (error%)
bm_chaos.py                  291.16 ->     294.88 :      +3.72 =  +1.278% (+/-0.00%)
bm_fannkuch.py                71.41 ->      72.37 :      +0.96 =  +1.344% (+/-0.00%)
bm_float.py                 4684.44 ->    4757.79 :     +73.35 =  +1.566% (+/-0.00%)
bm_hexiom.py                  34.28 ->      34.48 :      +0.20 =  +0.583% (+/-0.00%)
bm_nqueens.py               4075.03 ->    4148.72 :     +73.69 =  +1.808% (+/-0.00%)
bm_pidigits.py               627.13 ->     631.49 :      +4.36 =  +0.695% (+/-0.30%)
misc_aes.py                  336.71 ->     341.23 :      +4.52 =  +1.342% (+/-0.01%)
misc_mandel.py              2953.11 ->    2988.60 :     +35.49 =  +1.202% (+/-0.00%)
misc_pystone.py             1851.98 ->    1866.69 :     +14.71 =  +0.794% (+/-0.00%)
misc_raytrace.py             294.30 ->     297.65 :      +3.35 =  +1.138% (+/-0.01%)
viper_call0.py               576.58 ->     576.42 :      -0.16 =  -0.028% (+/-0.07%)
viper_call1a.py              549.99 ->     549.88 :      -0.11 =  -0.020% (+/-0.05%)
viper_call1b.py              434.32 ->     434.26 :      -0.06 =  -0.014% (+/-0.13%)
viper_call1c.py              439.14 ->     439.26 :      +0.12 =  +0.027% (+/-0.06%)
viper_call2a.py              535.30 ->     535.70 :      +0.40 =  +0.075% (+/-0.16%)
viper_call2b.py              377.01 ->     377.10 :      +0.09 =  +0.024% (+/-0.04%)

If anything there is a small improvement in performance (although that's probably just due to a slightly different layout in flash of the VM code).

@dpgeorge
Copy link
Member Author

Merged in 16f6169 and 08c1fe5

@dpgeorge dpgeorge closed this Aug 28, 2019
@dpgeorge dpgeorge deleted the py-vm-fix-reraise-traceback branch August 28, 2019 02:34
tannewt added a commit to tannewt/circuitpython that referenced this pull request Jul 21, 2021
Moved global variables to support multiple RP2040 PulseOuts
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.

None yet

1 participant