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

[qstr] Separate hash and len from string data #4583

Merged
merged 1 commit into from
Apr 12, 2021

Conversation

tyomitch
Copy link
Collaborator

@tyomitch tyomitch commented Apr 9, 2021

This allows the compiler to merge strings: e.g. "update",
"difference_update" and "symmetric_difference_update"
will all point to the same memory.

Shaves ~1KB off the image size, and potentially allows
bigger savings if qstr attrs are initialized in qstr_init(),
and not stored in the image.

This allows the compiler to merge strings: e.g. "update",
"difference_update" and "symmetric_difference_update"
will all point to the same memory.

Shaves ~1KB off the image size, and potentially allows
bigger savings if qstr attrs are initialized in qstr_init(),
and not stored in the image.
@dhalbert dhalbert requested review from jepler and tannewt April 9, 2021 16:02
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool! It all looks good to me. Thank you!

@jepler
Copy link
Member

jepler commented Apr 9, 2021

It's safe to say I don't entirely follow what has been done, but it gained us 200 bytes on trinket_m0 and apparently it works (I didn't test it for myself). It might be worth offering this change to MicroPython as well, if you feel like it.

@tannewt tannewt merged commit bd5a3a3 into adafruit:main Apr 12, 2021
tyomitch added a commit to tyomitch/circuitpython that referenced this pull request May 3, 2021
This allows the compiler to merge strings: e.g. "update",
"difference_update" and "symmetric_difference_update"
will all point to the same memory.

No functional change.

```
   bare-arm:   -12 -0.021%
minimal x86:   +38 +0.026% [incl +40(data)]
   unix x64:  -576 -0.114% [incl +32(data)]
unix nanbox:  -696 -0.158%
      stm32: -1336 -0.344% PYBV10
     cc3200:  -440 -0.240%
    esp8266: -1092 -0.159% GENERIC
      esp32: -1332 -0.093% GENERIC[incl -1408(data)]
        nrf:  -404 -0.273% pca10040
        rp2: -1024 -0.213% PICO
       samd:  -196 -0.190% ADAFRUIT_ITSYBITSY_M4_EXPRESS
```

Originally at adafruit#4583

Signed-off-by: Artyom Skrobov <tyomitch@gmail.com>
tyomitch added a commit to tyomitch/circuitpython that referenced this pull request May 6, 2021
This allows the compiler to merge strings: e.g. "update",
"difference_update" and "symmetric_difference_update"
will all point to the same memory.

No functional change.

```
   bare-arm:   -12 -0.021%
minimal x86:   +38 +0.026% [incl +40(data)]
   unix x64:  -576 -0.114% [incl +32(data)]
unix nanbox:  -696 -0.158%
      stm32: -1336 -0.344% PYBV10
     cc3200:  -440 -0.240%
    esp8266: -1092 -0.159% GENERIC
      esp32: -1332 -0.093% GENERIC[incl -1408(data)]
        nrf:  -404 -0.273% pca10040
        rp2: -1024 -0.213% PICO
       samd:  -196 -0.190% ADAFRUIT_ITSYBITSY_M4_EXPRESS
```

Originally at adafruit#4583

Signed-off-by: Artyom Skrobov <tyomitch@gmail.com>
@tyomitch
Copy link
Collaborator Author

It might be worth offering this change to MicroPython as well, if you feel like it.

micropython#7209 pending review since May 3rd -- would anybody like to help them review it?

dpgeorge pushed a commit to micropython/micropython that referenced this pull request Feb 11, 2022
This allows the compiler to merge strings: e.g. "update",
"difference_update" and "symmetric_difference_update" will all point to the
same memory.

No functional change.

The size reduction depends on the number of qstrs in the build.  The change
this commit brings is:

   bare-arm:    -4 -0.007%
minimal x86:  +150 +0.092% [incl +48(data)]
   unix x64:  -608 -0.118%
unix nanbox:  -572 -0.126% [incl +32(data)]
      stm32: -1392 -0.352% PYBV10
     cc3200:  -448 -0.244%
    esp8266: -1208 -0.173% GENERIC
      esp32: -1028 -0.068% GENERIC[incl -1020(data)]
        nrf:  -440 -0.252% pca10040
        rp2: -1072 -0.217% PICO
       samd:  -368 -0.264% ADAFRUIT_ITSYBITSY_M4_EXPRESS

Performance is also improved (on bare metal at least) for the
core_import_mpy_multi.py, core_import_mpy_single.py and core_qstr.py
performance benchmarks.

Originally at adafruit#4583

Signed-off-by: Artyom Skrobov <tyomitch@gmail.com>
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

3 participants