Skip to content

Moved speedups to a separate package. #77

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

Closed
wants to merge 1 commit into from
Closed

Moved speedups to a separate package. #77

wants to merge 1 commit into from

Conversation

jaraco
Copy link

@jaraco jaraco commented Mar 9, 2014

This pull request is designed first and foremost as a springboard for a discussion about a possible alternative to using the setuptools Feature functionality. The change extracts the C extensions into a SQLAlchemy-speedups project (which I've temporarily hosted at jaraco/sqlalchemy-speedups and published to PyPI). This technique allows the extensions to be indicated by requiring SQLAlchemy[speedups] or by installing the speedups package explicitly.

I am encouraged by this change. It was relatively easy to enact and it makes the code much cleaner. Suddenly, SQLAlchemy is a pure-Python package with much simpler build and packaging semantics, yet with a straightforward way to achieve the performance gains. It also means that the speedups package can be released and distributed separately, possibly as wheels or eggs or other platform-specific formats and only that package need deal with the complexities of the compile and build process.

Furthermore, by decoupling the aspects as separate packages, a deployment manager can easily detect the presence or absence of the speedups (even outside of the Python ecosystem), which would be harder to do with the aspect selected at build time.

I acknowledge that this change does require a bit of shift in perception about how SQLAlchemy is referenced and installed, and that's a non-trivial shift. That is, there's no way for the 'speedups' extra to be indicated by default and specifically contra-indicated (opt-out). Therefore, implementers would need to be instructed to always include [speedups] unless they were desired to be omitted.

I do believe that following this technique simplifies the build process and does move the packaging in a direction that's more consistent with the simpler build processes being developed by the PyPA (though I couldn't say specifically why; I only suspect based on the essence of the decoupling).

I'm eager to hear your feedback on this approach. Is it viable for SQLAlchemy? If so, what sort of timeline might one expect for adoption?

You're welcome to accept this pull request or re-implement the concept in your own style. I'm also happy to give up ownership of the SQLAlchemy-speedups name if that's the name you choose to adopt. I merely wanted to draft a fully-functional prototype to aid in your consideration.

@zzzeek
Copy link
Owner

zzzeek commented Mar 9, 2014

I'll think about it some more but I'm pretty sure I wouldn't be comfortable not having the C speedups install by default. SQLAlchemy has always been a flashpoint for speed complaints and a tremendous amount of effort was put into those C extensions to put to rest a whole series of complaints about our performance. It's been my observation that when SQLAlchemy or one of it's depending DBAPIs is doing something inefficiently, even if we've explicitly documented particular issues or caveats, users instead throw up their hands, being tweeting/blogging how "bloated" and "slow" SQLAlchemy is and begin recommending that we are to be avoided (see pandas-dev/pandas#2717 (comment) for an example of one user with speed issues that were not reported to us encouraging the pandas project to dump us for which I was very fortunately alerted. I've since improved the C extensions along with some adjustments to the cx_oracle dialect to fix this user's issue).

As far as the need to drop "Feature" from setuptools, I will just go the simple route and use an environment variable to indicate not installing the C extensions. The use case of not installing them when they are installable is really limited to specific testing scenarios. My concern about "Feature" is more oriented towards the fact that all the older SQLAlchemy versions that have "Feature" stuck in setup.py would no longer be installable via setuptools if/when "Feature" is removed; the setup.py will degrade silently to disutils due to the ImportError but this might be confusing/surprising to users who see SQLAlchemy not installing in the expected way.

@jaraco
Copy link
Author

jaraco commented Mar 10, 2014

Good points. Indeed, every time I've used extras, I've felt like the inclusion/exclusion model was inverted from my optimal use case. I've opened setuptools 163 to see if that aspect can't be improved. In the meantime, I'll close this PR.

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.

2 participants