Skip to content

Disable speedups for GraalPython#278

Merged
davidism merged 1 commit intopallets:mainfrom
msimacek:main
Feb 16, 2022
Merged

Disable speedups for GraalPython#278
davidism merged 1 commit intopallets:mainfrom
msimacek:main

Conversation

@msimacek
Copy link
Copy Markdown
Contributor

Disable speedups module for GraalPython as it doesn't work and wouldn't provide any speedups anyway. CC @timfel

Fixes #277

Checklist:

  • Add tests that demonstrate the correct behavior of the change. Tests should fail without the change.
    GraalPython doesn't yet work out of the box with tox. Since there is no testenv for Jython either, I hope that's fine.
  • Add or update relevant docs, in the docs folder and in code.
  • Add an entry in CHANGES.rst summarizing the change and linking to the issue.
  • Add .. versionchanged:: entries in any relevant code docs.
  • Run pre-commit hooks and fix any issues.
  • Run pytest and tox, no tests failed.

@davidism
Copy link
Copy Markdown
Member

I've considered changing this to "only compile when cpython", but I'm really not sure what proportion of implementations can or can't use speedups at this point.

@davidism davidism added this to the 2.1.0 milestone Feb 15, 2022
@msimacek
Copy link
Copy Markdown
Contributor Author

I don't know which other imlementations support the speedups, but I think in general "only compile when CPython" sounds better to me. Because if you don't use the speedups on an interpreter that could support them, the result is just a small slowdown. OTOH if you use the speedups on an interpreter that doesn't support them, it can crash. So flipping the condition to be CPython-only sounds safer. Do you want me to do it?

@davidism
Copy link
Copy Markdown
Member

I'm going to merge this as-is for now. I want to understand what implementations are out there before I commit to a bigger change.

@davidism davidism merged commit 6c17ee8 into pallets:main Feb 16, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

GraalPython should be excluded from C speedups module

2 participants