Skip to content
This repository has been archived by the owner on Nov 4, 2019. It is now read-only.

#9 - Performance issues #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

#9 - Performance issues #10

wants to merge 1 commit into from

Conversation

jcklie
Copy link
Contributor

@jcklie jcklie commented Sep 14, 2018

  • Use dictionary instead of list for feature loopup
  • Use a set instead of a list for ids

Before:

⇒  time python  sandbox.py
python sandbox.py  39,61s user 0,21s system 99% cpu 39,822 total

After:

⇒  time python  sandbox.py
python sandbox.py  4,85s user 0,09s system 99% cpu 4,947 total

- Use dictionary instead of list for feature loopup
- Use a set instead of a list for ids
@jcklie jcklie requested a review from reckart September 14, 2018 12:49
@reckart
Copy link
Member

reckart commented Sep 16, 2018

I don't understand the python versioning scheme... for me, a version increase is done when a release is being performed. Is merging a PR automatically a release? What happens when we have more than 26 PRs - does it go "0.1.1aa" then?

Copy link
Member

@reckart reckart left a comment

Choose a reason for hiding this comment

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

Looks ok. Is there anything special that should be done about ensuring that the dictionary has a "predicable" order (in Java terms LinkedHashMap vs HashMap)?

@reckart reckart added the 🆕Enhancement New feature or request label Sep 16, 2018
@reckart reckart added this to the 0.1.1 milestone Sep 16, 2018
@reckart reckart added ⚙️Refactoring and removed 🆕Enhancement New feature or request labels Sep 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants