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

Standardize the signature for updateFromExternalObject #30

Open
jamadden opened this issue Sep 21, 2017 · 1 comment
Open

Standardize the signature for updateFromExternalObject #30

jamadden opened this issue Sep 21, 2017 · 1 comment

Comments

@jamadden
Copy link
Member

The hot code path in internalization checks to see if update.updateFromExternalObject wants a "dataserver" keyword or a "context" or nothing. This has a few problems:

  1. Checking the argspec is insanely slow. In one test, it was 5.28us, vs checking an isinstance which clocked in at around 200ns. That's 26x slower. (I was able to get rid of argspec there, but I don't know if I can do that here while preserving compatibility.)
  2. A dataserver argument doesn't make any sense.
  3. There's a fallback path that passes nothing if we can't be exactly sure what the signature is.

This all needs to be simplified, starting with dropping the dataserver argument, and then maybe requiring everyone to accept a context argument.

@jamadden
Copy link
Member Author

jamadden commented Jul 9, 2018

We can cache this information based on the type of the object. We could then issue warnings for each broken type (where broken means 'needing dataserver or 'not accepting context'). This is similar to what we do for __ext_ignore_toExternalObject__.

Note that the main datastructures, compiled with cython, do not work with inspect at all, currently. (But see the mailing list discussion about PEP580.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant