Skip to content

py/builtinimport.c: Refactor module importing. #8010

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

Closed
wants to merge 1 commit into from

Conversation

jimmo
Copy link
Member

@jimmo jimmo commented Nov 18, 2021

This is just the refactor part of #5701 (but with more improvements).

Simplify and document/comment the handling of builtin import for:

  • already-loaded modules
  • built-in modules
  • built-in umodules (formerly weak links)
  • filesystem modules

Retains existing functionality with smaller code size but should also facilitate potential new features (built-in packages, controlling the frozen path).

Also makes the (unix-only) -m behavior a bit more obvious.

Code size reduction of 208 bytes on PYBV11.

@dpgeorge
Copy link
Member

Bare-arm is +16 with this... I guess most of the savings on PYBV11 come from disabling the "-m" behaviour.

Also appveyor fails, maybe because "-m" needs to be enabled on that port?

@dpgeorge dpgeorge added the py-core Relates to py/ directory in source label Nov 18, 2021
@jimmo jimmo force-pushed the import-refactor branch 3 times, most recently from e3d9fbc to b0ab887 Compare November 19, 2021 00:21
Simplify and document/comment the handling of builtin import for:
 - already-loaded modules
 - built-in modules
 - built-in umodules (formerly weak links)
 - filesystem modules

Retains existing functionality with smaller code size but should also
facilitate potential new features (built-in packages, controlling the
frozen path).

Also makes the (unix-only) -m behavior a bit more obvious.

Code size reduction of 208 bytes on PYBV11.
@jimmo
Copy link
Member Author

jimmo commented Nov 19, 2021

bare-arm is back to 0 now. Savings come from some small optimisations in the relative path resolving but yeah mostly the -m bit.

Windows build is fixed -- this was a logic error in weak link handling that the uasyncio tests being skipped triggered. I will add a test for this.

@dpgeorge
Copy link
Member

dpgeorge commented Dec 1, 2021

I double checked the coverage change locally, and there is no change to py/builtinimport.c.

Code size diff is:

   bare-arm:    +0 +0.000% 
minimal x86:   -64 -0.039% 
   unix x64:   -32 -0.006% 
unix nanbox:    -4 -0.001% 
      stm32:  -184 -0.047% PYBV10
     cc3200:  -120 -0.065% 
    esp8266:  -228 -0.033% GENERIC
      esp32:  -268 -0.018% GENERIC[incl +16(data)]
        nrf:  -152 -0.087% pca10040
        rp2:  -256 -0.052% PICO
       samd:   -80 -0.057% ADAFRUIT_ITSYBITSY_M4_EXPRESS

@dpgeorge
Copy link
Member

dpgeorge commented Dec 1, 2021

Merged in a7fa18c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
py-core Relates to py/ directory in source
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants