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

Enable cython caching #15430

Closed
robertwb opened this issue Nov 17, 2013 · 20 comments
Closed

Enable cython caching #15430

robertwb opened this issue Nov 17, 2013 · 20 comments

Comments

@robertwb
Copy link
Contributor

The entire Sage library fits into about 25MB (compressed) cache.

Component: build

Author: Robert Bradshaw

Branch/Commit: u/vbraun/ticket/15430 @ 064791b

Reviewer: Volker Braun

Issue created by migration from https://trac.sagemath.org/ticket/15430

@robertwb robertwb added this to the sage-6.0 milestone Nov 17, 2013
@robertwb
Copy link
Contributor Author

Branch: u/robertwb/ticket/15430

@robertwb
Copy link
Contributor Author

Author: Robert Bradshaw

@robertwb
Copy link
Contributor Author

New commits:

[c93d084](https://github.com/sagemath/sagetrac-mirror/commit/c93d084)Enable cython cache.

@robertwb
Copy link
Contributor Author

Commit: c93d084

@robertwb

This comment has been minimized.

@vbraun
Copy link
Member

vbraun commented Nov 18, 2013

Reviewer: Volker Braun

@vbraun
Copy link
Member

vbraun commented Nov 18, 2013

comment:3

Dependency checking seems to work for me. If there is any problem left then I'm pretty sure we'll only be able to find it if we use this seriously. So I'm in favor of pushing this into the sage-git release....

@vbraun
Copy link
Member

vbraun commented Nov 27, 2013

comment:4

Building Sage with a non-existing DOT_SAGE breaks because of this ticket. From the buildbot:

Compiling sage/ext/interpreters/wrapper_rdf.pyx because it changed.
Compiling sage/ext/interpreters/wrapper_cdf.pyx because it changed.
Compiling sage/ext/interpreters/wrapper_rr.pyx because it changed.
Compiling sage/ext/interpreters/wrapper_py.pyx because it changed.
Compiling sage/ext/interpreters/wrapper_el.pyx because it changed.
Traceback (most recent call last):
  File "setup.py", line 531, in <module>
    force=force)
  File "/home/buildbot/build/sage/plantain/sage_git/build/local/lib/python2.7/site-packages/Cython/Build/Dependencies.py", line 740, in cythonize
    os.mkdir(options.cache)
OSError: [Errno 2] No such file or directory: '/home/buildbot/build/sage/plantain/sage_git/dot_sage/cycache'

@vbraun
Copy link
Member

vbraun commented Nov 27, 2013

Changed branch from u/robertwb/ticket/15430 to u/vbraun/ticket/15430

@vbraun
Copy link
Member

vbraun commented Nov 27, 2013

Changed commit from c93d084 to 064791b

@vbraun
Copy link
Member

vbraun commented Nov 27, 2013

comment:6

Hmm the commit field was not automatically updated?


New commits:

[064791b](https://github.com/sagemath/sagetrac-mirror/commit/064791b)only enable cycache if DOT_SAGE exists

@vbraun
Copy link
Member

vbraun commented Nov 27, 2013

comment:7

please review my change...

@ohanar
Copy link
Member

ohanar commented Nov 27, 2013

comment:8

Why use a sage specific cycache directory? Why not something like ~/.cycache? (IMHO, Cython should use a default directory if Cython.Compiler.Main.default_options['cache'] is True).

Also, I think it should be togglable by an environment variable, say DISABLE_CYCACHE, in case someone doesn't want to use (e.g. for testing purposes/cython development purposes).

@vbraun
Copy link
Member

vbraun commented Nov 27, 2013

comment:9

The ccache spkg also uses $DOT_SAGE/ccache.

With this patch you can disable caching by passing an invalid directory. Ugly but yet another undocumented environment variable isn't that great either.

Supporting boolean Cython.Compiler.Main.default_options['cache'] and/or using os.makedirs() instead of mkdir are just Cython enhancement requests, maybe you want to post that to the Cython trac?

@robertwb
Copy link
Contributor Author

comment:10

This is quite the ugly hack, but should work. We can roll this back once cython/cython@b24ea41 is released.

@jdemeyer
Copy link

comment:12

I assume the intent of this patch was to always enable cycache by default? Because that's not what this patch does. It only works if the cycache directory already exists. Testing for the existence of X/.. is (almost) equivalent to testing for the existence of X.

@jdemeyer
Copy link

comment:13

Replying to @vbraun:

The ccache spkg also uses $DOT_SAGE/ccache.

That's not actually true. Sage doesn't set the ccache directory.

@jdemeyer
Copy link

comment:14

Replying to @robertwb:

This is quite the ugly hack, but should work. We can roll this back once cython/cython@b24ea41 is released.

I guess this is in Sage now, right?

@vbraun
Copy link
Member

vbraun commented Apr 11, 2014

comment:15

Yes, feel free to open a new ticket.

@jdemeyer
Copy link

comment:16

See #16148.

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

4 participants