Skip to content

Initial support for dict data structure #975

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

Merged
merged 6 commits into from
Aug 17, 2022
Merged

Conversation

czgdp1807
Copy link
Collaborator

@czgdp1807 czgdp1807 commented Aug 15, 2022

Closes #964

Following is the plan,

  1. Represent dict with the help of a struct in LLVM. Maintain original key-value pairs for the purpose of collision handling and in case a collision happens then distinguishing between update and insert operation. We need original key-value pairs anyways for all kinds of collision handling strategy.
  2. If collision happens while insertion then do linear probing (simple as well as cache efficient).
  3. Resizing strategy - All-at-once resizing strategy. Will happen if load factor touches a threshold value. Simpler than lazy resizing in which don't copy all the older key-value pairs into new array but instead only when they are accessed. Its a bit complicated to implement so I will do it in future.

TODOs after this PR - #983 (comment)

@czgdp1807
Copy link
Collaborator Author

@certik This is ready for review. I have mentioned my TODOs after this PR in #983 (comment). You can review each commit individually if you want. Let me know if anything needs a change here.

@czgdp1807 czgdp1807 marked this pull request as ready for review August 17, 2022 15:09
Copy link
Contributor

@certik certik left a comment

Choose a reason for hiding this comment

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

I reviewed every commit, everything looks great.

My only possible question is if the CPython version of the test takes a long time, if so, we should reduce the number of loop iterations in the test.

That can be adjusted later.

So +1 to merge.

Great job!

@certik certik merged commit 61b307e into lcompilers:main Aug 17, 2022
@certik
Copy link
Contributor

certik commented Aug 17, 2022

@czgdp1807 is this ordered or unordered dictionary?

@czgdp1807
Copy link
Collaborator Author

czgdp1807 commented Aug 17, 2022

I think this is unordered dictionary (because hash maps are unordered in nature). For ordered dictionary we will require an implementation similar to std::map.

Though you can make ordered dictionary out of unordered ones by subclassing and tracking the insertion order of keys. I would keep both ordered and unordered ones as one has some advantages over the other.

@certik
Copy link
Contributor

certik commented Aug 17, 2022

Here is another way to implement OrderedDict: https://github.com/Aqcurate/OrderedDict#ordered-dictionary-implementation

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.

Supportding dict in LLVM backend
2 participants