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

Consider Cythonizing When compiling #12338

Open
trypolis464 opened this issue Apr 25, 2021 · 5 comments
Open

Consider Cythonizing When compiling #12338

trypolis464 opened this issue Apr 25, 2021 · 5 comments

Comments

@trypolis464
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Not necessarily a problem, but it certainly would make NVDA run faster, possibly also allowing us to remove certain things from nvdaHelper that needed it for the speed.

Describe the solution you'd like

Cythonize the NVDA codebase when building, thus majorly improving speed.

Describe alternatives you've considered

Just keep it the way it is is the only one I can think of. As I said, it's not necessarily a problem, but would be interesting and nice if it could happen.

Additional context

There's a tool for Python, called Cython. Pip install Cython. It turns your Python modules into native C DLLs that Python can handle, with a file extension .pyd. Very similar to the .pyc files, but native C. Having all this be compiled to C code when running would improve speed by a lot. I'm not sure how Cython interfaces with SCons, however.

@josephsl
Copy link
Collaborator

josephsl commented Apr 25, 2021 via email

@lukaszgo1
Copy link
Contributor

I believe this has been discussed with @michaelDCurran during one of the NVDA conferences where he said that either this has been attempted and speed improvements were negligible or he don't believe it would make NVDA faster. Obviously the best way to measure this would be to actually try Cythonizing and see what happens.

@JulienCochuyt
Copy link
Collaborator

Another point to consider is how cythonized modules behave in the context of monkey-patching as quite a few addons use it for different purposes.

@LeonarddeR
Copy link
Collaborator

I did some research into this and cythonizing the whole codebase takes a massive amount of time. This would be quite problematic with our current workflow.

@ethindp
Copy link

ethindp commented May 9, 2021

@LeonarddeR It may take a while, but it could most likely be automated. What exactly has to be done? Cython can take raw Python input and can generate code from that; my preferred invocation is something like: cython --embed -3 --capi-reexport-cincludes --fast-fail -Werror -Wextra for release or cython --embed -3 --capi-reexport-cincludes --fast-fail -Werror -Wextra --gdb for debug.
I'd strongly encourage that this be done. The architecture issue, as pointed out by @josephsl, is not actually a problem because Cython only generates C code and does not actually compile anything. The visual studio tools would take care of generating the architecture-specific code.
Monkey-patching also shouldn't be a problem, I don't think. Cython will still embed the Python interpreter since that's required to even run Python modules.
Finally, as for performance, the only way to actually see if there's a gain would be to try it using the highest optimization settings. The dependencies might be a problem, but I've never been a fan of the fact that the NVDA source code repository ships with (already compiled) libraries. I understand that this is to make it easier to run snapshots or the code itself, but that isn't much of a justification. If your so eager to run the bleeding edge code, then its reasonable for the developers to expect you to know how to set up and build all the necessary dependencies on your own. vcpkg would probably help with them, though we'd need to push ports to it for those that it doesn't have.
Hope this isn't harsh or anything.

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

6 participants