Opened 4 years ago

Closed 3 years ago

#21509 closed enhancement (fixed)

Improve Cython debugging

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-8.2
Component: build Keywords:
Cc: fbissey Merged in:
Authors: Jeroen Demeyer Reviewers: François Bissey
Report Upstream: N/A Work issues:
Branch: fa789c2 (Commits) Commit: fa789c2793dda97129ad7c7839a636bf6be29c60
Dependencies: Stopgaps:

Description (last modified by jdemeyer)

  1. Store cython_debug in site-packages/sage/cython_debug instead of in the Cython build directory.
  1. Upgrade cysignals:

Tarball: https://pypi.python.org/packages/92/15/9ca4a50304fca8debaccac2e9b630626c13f4c91992ca5a86f52732bb983/cysignals-1.6.9.tar.gz#md5=d8155169fefb8ea9c036c2a8ee1495bd

Change History (33)

comment:1 Changed 4 years ago by mkoeppe

  • Milestone changed from sage-7.4 to sage-7.5

comment:2 Changed 4 years ago by fbissey

  • Description modified (diff)

comment:3 follow-up: Changed 4 years ago by mkoeppe

Can you explain what 'cython_debug' is used for precisely, in the description? (Or point to a reference?)

comment:4 in reply to: ↑ 3 Changed 4 years ago by jdemeyer

Replying to mkoeppe:

Can you explain what 'cython_debug' is used for precisely, in the description? (Or point to a reference?)

It is used by https://github.com/sagemath/cysignals/blob/master/src/scripts/cysignals-CSI-helper.py#L43

comment:5 in reply to: ↑ description Changed 4 years ago by jdemeyer

Replying to mkoeppe:

fbissey proposes to install cython_debug somewhere in SAGE_LOCAL because it is needed for debugging at runtime.

See https://trac.sagemath.org/ticket/21480#comment:127 where I question this proposal.

comment:6 Changed 4 years ago by jdemeyer

  • Description modified (diff)
  • Summary changed from Install cython_debug somewhere in SAGE_LOCAL to Install cython_debug somewhere else

comment:7 Changed 3 years ago by jdemeyer

  • Dependencies set to #24690
  • Description modified (diff)

comment:8 Changed 3 years ago by jdemeyer

  • Summary changed from Install cython_debug somewhere else to Install cython_debug in site-packages/sage

comment:9 Changed 3 years ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/21509

comment:10 Changed 3 years ago by jdemeyer

  • Authors set to Jeroen Demeyer
  • Commit set to 3e9996321c6d20a129efaf0b80b84ca89d0c3ad4
  • Description modified (diff)
  • Summary changed from Install cython_debug in site-packages/sage to Improve Cython debugging

New commits:

1bf128eInstall the .pyx files
996759cNo longer add SAGE_SRC to sys.path
3e99963Install cython_debug in site-packages/sage

comment:11 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:12 Changed 3 years ago by jdemeyer

  • Description modified (diff)
  • Milestone changed from sage-7.5 to sage-8.2

comment:13 Changed 3 years ago by git

  • Commit changed from 3e9996321c6d20a129efaf0b80b84ca89d0c3ad4 to edfebbd27e03df232d50a36f4a59a52c941586ea

Branch pushed to git repo; I updated commit sha1. New commits:

76248ffFix https://github.com/cython/cython/pull/2095
edfebbdUpgrade cysignals to version 1.6.7

comment:14 Changed 3 years ago by jdemeyer

  • Status changed from new to needs_review

comment:15 Changed 3 years ago by fbissey

  • Reviewers set to François Bissey
  • Status changed from needs_review to positive_review

Let's make some progress here. It needs to go to the bots now.

comment:16 follow-up: Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work

Tests fail on OSX

[cysignals-1.6.7] Successfully installed cysignals-1.6.7
[cysignals-1.6.7] Cleaning up...
[cysignals-1.6.7] python -B rundoctests.py src/cysignals/*.pyx
[cysignals-1.6.7] cd example && python setup.py clean build
[cysignals-1.6.7] sigaltstack: Cannot allocate memory
[cysignals-1.6.7] Doctesting 5 files.
[cysignals-1.6.7] make[3]: *** [check-doctest] Error 1
[cysignals-1.6.7] Compiling cysignals_example.pyx because it changed.
[cysignals-1.6.7] [1/1] Cythonizing cysignals_example.pyx
[cysignals-1.6.7] running clean
[cysignals-1.6.7] running build
[cysignals-1.6.7] running build_ext
[cysignals-1.6.7] building 'cysignals_example' extension
[cysignals-1.6.7] creating build
[cysignals-1.6.7] creating build/temp.macosx-10.9-x86_64-2.7
[cysignals-1.6.7] gcc -fno-strict-aliasing -g -O2 -DNDEBUG -g -fwrapv -O3 -Wall -Wno-unused -I/Users/buildslave-sage/slave/sage_git/build/local/lib/python2.7/site-packages/cysignals -I/Users/buildslave-sage/slave/sage_git/build/local/include/python2.7 -c cysignals_example.cpp -o build/temp.macosx-10.9-x86_64-2.7/cysignals_example.o
[cysignals-1.6.7] creating build/lib.macosx-10.9-x86_64-2.7
[cysignals-1.6.7] g++ -bundle -undefined dynamic_lookup -L/Users/buildslave-sage/slave/sage_git/build/local/lib -Wl,-rpath,/Users/buildslave-sage/slave/sage_git/build/local/lib -L/Users/buildslave-sage/slave/sage_git/build/local/lib -Wl,-rpath,/Users/buildslave-sage/slave/sage_git/build/local/lib build/temp.macosx-10.9-x86_64-2.7/cysignals_example.o -L/Users/buildslave-sage/slave/sage_git/build/local/lib -o build/lib.macosx-10.9-x86_64-2.7/cysignals_example.so -lpari
[cysignals-1.6.7] make[3]: Target `check-install' not remade because of errors.
[cysignals-1.6.7] 
[cysignals-1.6.7] real	0m9.707s
[cysignals-1.6.7] user	0m8.247s
[cysignals-1.6.7] sys	0m1.497s
[cysignals-1.6.7] ************************************************************************
[cysignals-1.6.7] Error testing package cysignals-1.6.7
[cysignals-1.6.7] ************************************************************************

comment:17 in reply to: ↑ 16 Changed 3 years ago by jdemeyer

Replying to vbraun:

Tests fail on OSX

[cysignals-1.6.7] sigaltstack: Cannot allocate memory

That's really strange...

comment:18 Changed 3 years ago by jdemeyer

I see, OS X requires a minimum stack size of 215 bytes compared to 211 on Linux and I'm using a hardcoded value of 214.

comment:19 Changed 3 years ago by jdemeyer

Fixing the stack size is easy but it's not the only problem. The stack overflow test raises SIGILL for some reason...

comment:20 Changed 3 years ago by jdemeyer

Strange... it seems that test_stack_overflow() works in the main process but not in a forked child process. Why should that even matter?

comment:21 Changed 3 years ago by jdemeyer

As far as I can tell, this is an OS X bug... sigaltstack doesn't survive after a fork. It's as if sigaltstack was never run.

Last edited 3 years ago by jdemeyer (previous) (diff)

comment:22 Changed 3 years ago by jdemeyer

Ugh... even after working around that, there are still problems with sigaltstack. Once it's on the alt stack, it is stuck running there somehow.

comment:23 Changed 3 years ago by jdemeyer

Why is OS X implementing sigaltstack so differently? This even works on Solaris...

comment:24 Changed 3 years ago by jdemeyer

  • Dependencies #24690 deleted
  • Description modified (diff)

comment:25 Changed 3 years ago by git

  • Commit changed from edfebbd27e03df232d50a36f4a59a52c941586ea to 7d9e52f4dc50311de7fee1bbca675425a00a72cb

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

83f0d1cInstall cython_debug in site-packages/sage
7d9e52fUpgrade cysignals to version 1.6.8

comment:26 Changed 3 years ago by jdemeyer

  • Status changed from needs_work to positive_review

Fixed cysignals on OS X.

comment:27 Changed 3 years ago by jdemeyer

  • Status changed from positive_review to needs_work

comment:28 Changed 3 years ago by jdemeyer

  • Description modified (diff)

comment:29 Changed 3 years ago by git

  • Commit changed from 7d9e52f4dc50311de7fee1bbca675425a00a72cb to fa789c2793dda97129ad7c7839a636bf6be29c60

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

80aeca1Install cython_debug in site-packages/sage
fa789c2Upgrade cysignals to version 1.6.9

comment:30 Changed 3 years ago by jdemeyer

  • Status changed from needs_work to positive_review

New release of cysignals.

comment:31 Changed 3 years ago by vbraun

  • Status changed from positive_review to needs_work

See #21509

comment:32 Changed 3 years ago by vbraun

  • Status changed from needs_work to positive_review

comment:33 Changed 3 years ago by vbraun

  • Branch changed from u/jdemeyer/ticket/21509 to fa789c2793dda97129ad7c7839a636bf6be29c60
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.