Skip to content

py/map: Convert map implementation to preserve insertion order (WIP, RFC) - #6173

Open
dpgeorge wants to merge 1 commit into
micropython:masterfrom
dpgeorge:py-map-ordered
Open

py/map: Convert map implementation to preserve insertion order (WIP, RFC)#6173
dpgeorge wants to merge 1 commit into
micropython:masterfrom
dpgeorge:py-map-ordered

Conversation

@dpgeorge

@dpgeorge dpgeorge commented Jun 19, 2020

Copy link
Copy Markdown
Member

This PR changes the map implementation to make maps preserve insertion order, ie dicts are ordered by default. At this stage it's just for discussion, there's not (yet) any intention of merging it. See #6170 for additional discussion on this topic.

It uses a similar algorithm to PyPy/CPython. RAM overhead compared to the existing (non-order preserving) implementation is +12.5% for dicts with <255 elements, and +25% for dicts with <65535 elements. The code change is surprisingly simple.

TODO:

  • support dicts >65535 elements
  • make it optional at compile-time
  • make OrderedDict use this new implementation
  • implement more efficient deletion (currently there are pathological cases where the dict will grow forever if elements are continuously deleted and inserted)
  • profile performance and memory use

TODO:
- make it optional at compile-time
- make OrderedDict use this new implementation
- implement more efficient deletion
- profile performance and memory use

Signed-off-by: Damien George <damien@micropython.org>
@andrewleech

Copy link
Copy Markdown
Contributor

It's a much smaller/cleaner changeset than I'd expected. Once the existing OrderedDict implementation is removed I'd expect an overall flash saving.

@RayaneCTX

RayaneCTX commented Feb 23, 2022

Copy link
Copy Markdown
Contributor

Hi, another student and I wanted to create a pull request for dictionary union. Would it make sense for us to contribute to this pull request (e.g. addressing the TODOs) to have it eventually merged with master? Am I correct in assuming that there would be no point in creating a pull request for dictionary union without map ordering, or should we create the pull request anyways and have the discussion there (using sorted() inside the test to obviate to ordering issue for now)?

tannewt added a commit to tannewt/circuitpython that referenced this pull request Mar 21, 2022
…n-main

Translations update from Hosted Weblate
kbsriram added a commit to kbsriram/circuitpython that referenced this pull request Jul 15, 2023
Fixes adafruit#8173

It looks like a small fix, and mostly independent of upstream
plans around micropython#6173

I also filed an issue upstream
micropython#12011
dhalbert pushed a commit to dhalbert/circuitpython that referenced this pull request Jul 23, 2023
Fixes adafruit#8173

It looks like a small fix, and mostly independent of upstream
plans around micropython#6173

I also filed an issue upstream
micropython#12011
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.

3 participants