Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#15430 closed enhancement (fixed)

Enable cython caching

Reported by: robertwb Owned by:
Priority: major Milestone: sage-6.0
Component: build Keywords:
Cc: Merged in:
Authors: Robert Bradshaw Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: u/vbraun/ticket/15430 (Commits) Commit: 064791bac10bcb82d5bb663cd7f35b4f7945a0fd
Dependencies: Stopgaps:

Description (last modified by robertwb)

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

Change History (16)

comment:1 Changed 6 years ago by robertwb

  • Branch set to u/robertwb/ticket/15430
  • Created changed from 11/17/13 06:30:56 to 11/17/13 06:30:56
  • Modified changed from 11/17/13 06:30:56 to 11/17/13 06:30:56

comment:2 Changed 6 years ago by robertwb

  • Authors set to Robert Bradshaw
  • Commit set to c93d08403a56097ff06b7894d28013d50d813950
  • Description modified (diff)
  • Status changed from new to needs_review

New commits:

c93d084Enable cython cache.

comment:3 Changed 6 years ago by vbraun

  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

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....

comment:4 Changed 6 years ago by vbraun

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'

comment:5 Changed 6 years ago by vbraun

  • Branch changed from u/robertwb/ticket/15430 to u/vbraun/ticket/15430

comment:6 Changed 6 years ago by vbraun

  • Commit changed from c93d08403a56097ff06b7894d28013d50d813950 to 064791bac10bcb82d5bb663cd7f35b4f7945a0fd

Hmm the commit field was not automatically updated?


New commits:

064791bonly enable cycache if DOT_SAGE exists

comment:7 Changed 6 years ago by vbraun

  • Status changed from positive_review to needs_review

please review my change...

comment:8 Changed 6 years ago by ohanar

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).

comment:9 follow-up: Changed 6 years ago by vbraun

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?

comment:10 follow-up: Changed 6 years ago by robertwb

  • Status changed from needs_review to positive_review

This is quite the ugly hack, but should work. We can roll this back once https://github.com/cython/cython/commit/b24ea416d3658546c919d3ab8ba54e94eed187ba is released.

comment:11 Changed 6 years ago by vbraun

  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:12 Changed 6 years ago by jdemeyer

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.

comment:13 in reply to: ↑ 9 Changed 6 years ago by jdemeyer

Replying to vbraun:

The ccache spkg also uses $DOT_SAGE/ccache.

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

comment:14 in reply to: ↑ 10 Changed 6 years ago by jdemeyer

Replying to robertwb:

This is quite the ugly hack, but should work. We can roll this back once https://github.com/cython/cython/commit/b24ea416d3658546c919d3ab8ba54e94eed187ba is released.

I guess this is in Sage now, right?

comment:15 Changed 6 years ago by vbraun

Yes, feel free to open a new ticket.

comment:16 Changed 6 years ago by jdemeyer

See #16148.

Note: See TracTickets for help on using tickets.