Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#13865 closed enhancement (fixed)

Document that SAGE_DEBUG is three-state

Reported by: vbraun Owned by: mvngu
Priority: major Milestone: sage-5.6
Component: documentation Keywords:
Cc: leif Merged in: sage-5.6.beta2
Authors: Volker Braun Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Currently, SAGE_DEBUG is documented as

If it is unset (the default) or set to “yes”, then debugging is turned on.

This is fine if you just add gcc -g, but the utility of having a real Python debug build shows that there should be more to debugging than just adding symbols. However, this has a real performance impact and should not be the default. So I propose that SAGE_DEBUG have three possible values:

  • SAGE_DEBUG=no means no debugging symbols (no gcc -g), which mostly saves disk space.
  • SAGE_DEBUG=yes builds debug versions if possible (in particular, Python and Singular). These will be notable slower.
  • Anything else (including unset SAGE_DEBUG) is the same as the old default, compile with debugging symbols but no debugging options that influence performance.

Attachments (1)

trac_13865_SAGE_DEBUG_documentation.patch (1.6 KB) - added by jdemeyer 7 years ago.
Updated patch

Download all attachments as: .zip

Change History (22)

comment:1 Changed 7 years ago by vbraun

  • Authors set to Volker Braun
  • Status changed from new to needs_review

comment:2 Changed 7 years ago by vbraun

I've posted to sage-devel since this might be of interest to a larger audience: https://groups.google.com/d/topic/sage-devel/Lmo6mmDylj4/discussion

comment:3 Changed 7 years ago by nbruin

Ah, 'anything else' sounds like a great 3rd case.

Is there any standard on whether "yes/no", "Yes/No", "true/false", "on/off" etc. is preferred? I always end up having to google the environment variables and then hope and check later that the value I used had the desired effect.

comment:4 Changed 7 years ago by vbraun

I think all documented environment variables use yes/no for a binary setting. In Python we always use True/False since those are Python bools.

comment:5 Changed 7 years ago by leif

  • Cc leif added

Component: Documentation?

comment:6 Changed 7 years ago by vbraun

You want to file it under something else? It is just a doc patch.

comment:7 Changed 7 years ago by SimonKing

Is this ticket about fixing documentation of SAGE_DEBUG, or is it about proper behaviour/use of SAGE_DEBUG? If it is the latter, then I suggest one should also raise the singular-spkg from #13731 to a new patch level and replace/add the environment variable SINGULAR_XALLOC by SAGE_DEBUG.

comment:8 Changed 7 years ago by vbraun

I intended it just as a discussion and fix for the docstring. But if you want to add a new singular spkg then thats fine with me, too.

comment:9 Changed 7 years ago by leif

Replying to vbraun:

You want to file it under something else? It is just a doc patch.

Well, the documentation is certainly wrong (or not up-to-date), as the default is actually rather no. (We decided to add debug symbols by default since that doesn't have a performance impact -- except for larger files and hence probably slower start-up/loading, but one can strip these symbols later. SAGE_DEBUG=yes in contrast does -- depending on the spkg -- e.g. disable optimization, add extra code [probably with some runtime overhead] , so at least may have a major performance impact.)


W.r.t. the component: I just meant changing the meaning certainly extends to more than a documentation change (cf. the ticket's title; otherwise I'd call it "Clarify SAGE_DEBUG", say).

Actually, what you propose is IMHO what we already have (or shouldTM have)... ;-)

So just correcting or clarifying the documentation may indeed be sufficient. :-) (But I'd then change the ticket's title, as mentioned.)

comment:10 Changed 7 years ago by vbraun

  • Summary changed from Make SAGE_DEBUG three-state to Document that SAGE_DEBUG is three-state

comment:11 Changed 7 years ago by leif

Ok...

s/notable/notably/

s/to pinpoint/to e.g. pinpoint/

comment:12 Changed 7 years ago by leif

Not sure whether the Sage Developer's Guide (on spkgs) is up-to-date right now.

comment:13 Changed 7 years ago by leif

We should probably also mention that (or how) debug symbols can be stripped afterwards...

comment:14 follow-up: Changed 7 years ago by vbraun

Fixed. I grepped the docs and this is the only place that mentions SAGE_DEBUG. No idea how to strip besides doing it yourself, and that definitely shouldn't be covered in the install guide.

comment:15 in reply to: ↑ 14 Changed 7 years ago by leif

Replying to vbraun:

Fixed. I grepped the docs and this is the only place that mentions SAGE_DEBUG.

Oh. I thought it was at least in some spkg-installtemplate.


No idea how to strip besides doing it yourself, and that definitely shouldn't be covered in the install guide.

There used to be sage [-]-strip, but that got removed at some point because it did more than just stripping debug symbols (which also failed on some -- presumably MacOS X -- systems) IIRC.

(We now -- or still -- have the "micro-release" make target though, see also $SAGE_LOCAL/bin/sage-micro_release...)

comment:16 Changed 7 years ago by jdemeyer

The patch needs a proper commit message.

comment:17 Changed 7 years ago by vbraun

Fixed

comment:18 follow-up: Changed 7 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to positive_review

Fine for me. Actually changing packages to be consistent with this documentation should be done in separate tickets.

comment:19 in reply to: ↑ 18 Changed 7 years ago by leif

Replying to jdemeyer:

changing packages to be consistent with this documentation should be done in separate tickets.

Of course.

comment:20 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.6.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed

Changed 7 years ago by jdemeyer

Updated patch

comment:21 Changed 7 years ago by jdemeyer

Rebased to #13032.

Note: See TracTickets for help on using tickets.