Skip to content

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

Closed
jimmo wants to merge 1 commit into
micropython:masterfrom
jimmo:import-refactor
Closed

jimmo wants to merge 1 commit into
micropython:masterfrom
jimmo:import-refactor

Conversation

@jimmo

@jimmo jimmo commented Nov 18, 2021

Copy link
Copy Markdown
Member

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
Copy Markdown
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

jimmo commented Nov 19, 2021

Copy link
Copy Markdown
Member Author

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

dpgeorge commented Dec 1, 2021

Copy link
Copy Markdown
Member

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

dpgeorge commented Dec 1, 2021

Copy link
Copy Markdown
Member

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