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

RFC: OrderedDict v1 #2556

Closed
wants to merge 1 commit into from
Closed

Conversation

kmsquire
Copy link
Member

  • Includes tests
  • Updated OrderedDict, misc. other docs
  • Regenerated helpdb.jl

This version is slightly slower than the version at https://github.com/kmsquire/OrderedCollections.jl and discussed in #2548, but thanks to @JeffBezanson pointing out some flaws in my early attempts, it's cleaner and not much slower, and most importantly, it doesn't depend on the internal implementation of Dict.

Compared with Dict, it has ~40% insertion and ~50% deletion overhead.

The version at https://github.com/kmsquire/OrderedCollections.jl has a similar insertion overhead, and a ~10% deletion overhead.

I can submit the version at https://github.com/kmsquire/OrderedCollections.jl instead (which is a simple modification of Dict in julia/base), or make that one available via a package. Other suggested updates or improvements are welcome. (Done.)

EDIT:
I would appreciate any feedback on this version vs. the one based on AbstractHashDict in #2548. My own take is that this one is a bit cleaner, and that one is a bit faster, but somewhat invasive. @JeffBezanson expressed some concern about the other version earlier, but still seemed open to exploring it.

It may be the case that a doubly-linked list version of OrderedDict is better, so someone should feel free to contribute that as an alternative (or addition).

* Includes tests
* Updated OrderedDict, misc. other docs
* Regenerated helpdb.jl
@kmsquire
Copy link
Member Author

WIP, but I'm actually looking for constructive feedback on this and #2558. Otherwise, this should be complete.

@kmsquire
Copy link
Member Author

While they work, both this and the other OrderedDict version in #2548 are a little too complex. I'm working on a simpler version, so I'm going to withdraw them both for now, and submit the simpler version soon.

@kmsquire kmsquire closed this Mar 19, 2013
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.

1 participant