Opened 3 years ago

Closed 13 months 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 3 years ago by mkoeppe

  • Milestone changed from sage-7.4 to sage-7.5

comment:2 Changed 3 years ago by fbissey

  • Description modified (diff)

comment:3 follow-up: Changed 3 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 3 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 3 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 2 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 15 months ago by jdemeyer

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

comment:8 Changed 15 months ago by jdemeyer

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

comment:9 Changed 15 months ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/21509

comment:10 Changed 15 months 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 15 months ago by jdemeyer

  • Description modified (diff)

comment:12 Changed 15 months ago by jdemeyer

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

comment:13 Changed 15 months 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 15 months ago by jdemeyer

  • Status changed from new to needs_review

comment:15 Changed 14 months 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 14 months 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 14 months 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 14 months 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 14 months 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 14 months 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 14 months 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 14 months ago by jdemeyer (previous) (diff)

comment:22 Changed 14 months 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 14 months ago by jdemeyer

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

comment:24 Changed 14 months ago by jdemeyer

  • Dependencies #24690 deleted
  • Description modified (diff)

comment:25 Changed 14 months 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 14 months ago by jdemeyer

  • Status changed from needs_work to positive_review

Fixed cysignals on OS X.

comment:27 Changed 13 months ago by jdemeyer

  • Status changed from positive_review to needs_work

comment:28 Changed 13 months ago by jdemeyer

  • Description modified (diff)

comment:29 Changed 13 months 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 13 months ago by jdemeyer

  • Status changed from needs_work to positive_review

New release of cysignals.

comment:31 Changed 13 months ago by vbraun

  • Status changed from positive_review to needs_work

See #21509

comment:32 Changed 13 months ago by vbraun

  • Status changed from needs_work to positive_review

comment:33 Changed 13 months 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.