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

libcpp.map.map 'get' is 1e6 slower than libcpp.map.map '__getitem__' #1521

Open
ctb opened this issue Nov 12, 2016 · 3 comments
Open

libcpp.map.map 'get' is 1e6 slower than libcpp.map.map '__getitem__' #1521

ctb opened this issue Nov 12, 2016 · 3 comments

Comments

@ctb
Copy link

ctb commented Nov 12, 2016

With this code:

from libcpp.map cimport map

cdef class int_int_map:
     cdef public map[long, long] _values

     def __getitem__(self, k):
         return self._values[k]

     def __setitem__(self, k, v):
         self._values[k] = v

     def get(self, k):
         return self._values.get(k)

I find that for 200,000 dictionary entries, the 'get' function is about 1m times slower than getitem -- please see https://gist.github.com/ctb/ecea9809b8e1b6503abda1316b0de5b1 for a full working example. I'm sure I'm doing something wrong but I'm not sure what...

@scoder
Copy link
Contributor

scoder commented Nov 13, 2016

Without trying it, I guess that cython -a would show you that calling .get() on a C++ map copies the map into a Python dict, then calls .get() on it, then throws away the dict. C++ doesn't have that method.

Works as designed.

@scoder scoder closed this as completed Nov 13, 2016
@ctb
Copy link
Author

ctb commented Nov 13, 2016 via email

@robertwb robertwb reopened this Nov 16, 2016
@robertwb
Copy link
Contributor

Actually, this is quite dangerous. Makes sense for primitives like complex, but imagine

cdef vector[int] v = ...
v.pop()

I think we'd be better off with errors about missing methods here.

robertwb added a commit to robertwb/cython that referenced this issue Nov 16, 2016
In particular, this limits errors caused by mutating implicit temporary
objects, e.g.

  cdef vector[int] v = ...
  v.pop()

See also github issue cython#1521.
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

No branches or pull requests

3 participants