RFC: implement proper debug mode for packages (with support for re-precompilation) #37874

KristofferC · 2020-10-03T16:53:02Z

This adds a proper debug mode that packages can use. One of the crucial things is that the query for debug mode is available to use during precompilation and that Julia will create a new precompile cache when it changes. With the LRU cache system for .ji files one can swap between debug and non-debug mode freely without julia recompiling over and over.

This also disables the assertion macro in normal running code.

As an example: Loading a package with this content:

module FooBar

@show Base.isdebug()
err() = @assert false

end # module

in non-debug mode:

❯ julia --project -q
julia> using FooBar
[ Info: Precompiling FooBar [fad23a86-9c0b-4cc2-9d4f-c6fb60d9cd78]
Base.isdebug() = false

julia> FooBar.err() # assertion did not not fire

julia>

in debug mode:

❯ ~/julia/julia --project -q -g
julia> using FooBar
[ Info: Precompiling FooBar [fad23a86-9c0b-4cc2-9d4f-c6fb60d9cd78]
Base.isdebug() = true

julia> FooBar.err()
ERROR: AssertionError: false
Stacktrace:
 [1] err()
   @ FooBar ~/julia/FooBar/src/FooBar.jl:4
 [2] top-level scope
   @ REPL[2]:1

and then back to non debug mode (note no re-precompilation)

❯ ~/julia/julia --project -q
julia> using FooBar

julia> FooBar.err()

julia>

Questions (for triage):

Export isdebug?
Is it the -g flag we want to use to enable / disable debug mode for packages?
What about @assert in Base. We don't really want to ship two sysimages so maybe just leave it on there for now?
Is a true/false enough for debug level or should there be more fine-grained numbers?

TODO:

Enable @assert in stdlibs
Tests- Make Pkg.test run tests in debug mode

cc @tkf, @staticfloat

quinnj · 2020-10-03T17:01:31Z

This is super cool; could we make it a debug level::Int instead of Bool? I tend to find myself wanting to have debug code fenced for various "levels" and then being able to set the level to enable certain assertions/etc.

KristofferC · 2020-10-03T17:07:15Z

To me, that feels a bit excessive. Just enable everything if the debug level is on? It seems a bit like the Logging levels. Maybe we could have it take the same numerical value as the -g to julia? I added it to the questions to resolve.

quinnj · 2020-10-03T17:17:19Z

Yeah, I guess it does get close to "logging" terrority of functionality

tkf · 2020-10-03T21:58:22Z

a debug level::Int instead of Bool?

It should be possible to implement something like this with #37595 as a library.

Edit: reworded

base/loading.jl

KristofferC · 2020-10-04T07:19:20Z

I think something like this should be implementable with #37595 as a library.

Note that I don't want one to have to add a dependency and do changes in a project file to use debug mode.

KristofferC · 2020-10-04T11:28:30Z

base/error.jl

@@ -208,6 +202,7 @@ julia> @assert isodd(3) "What even are numbers?"
 ```
 """
 macro assert(ex, msgs...)
+    @isdefined(isdebug) && !(isdebug()) && return nothing


This is for the Compiler module where isdebug is not defined. Is this the best way?

options.jl is already included by this point during bootstrap. So you could just eliminate the julia_debug global and define isdebug() directly in error.jl as

isdebug() = JLOptions().debug_level >= 2

?

Yeah, but that ties isdebug directly to the debugging level (while it is not only tied to it in the sense that it is used to set the variable on startup). With the current implementation you can force julia debug mode without having to start julia itself with -g2 (maybe Pkg wants to do that in Pkg.test?). I don't know if the extra flexibility is worth it though.

Allowing julia_debug to change dynamically seems like it could be confusing and lead to inconsistencies between the precompiled code in different modules which are loaded into the session.

But maybe that's a reasonable tradeoff for flexibility, I'm not sure.

Example crazy thing:

module A if Base.isdebug() struct X x debug_field end else struct X x end end end

Now other modules which depend on A can't depend on the value of isdebug() to use X correctly (including macros defined by A and expanded in another module).

Allowing julia_debug to change dynamically seems like it could be confusing and lead to inconsistencies between the precompiled code in different modules which are loaded into the session.

Yeah, but julia_debug is internal so we'll just not change it dynamically in a way like that :P. But yes, I did think about changing this to exactly what you say. Can be discussed at triage as well. This problem already kind of exists in changing environments dynamically, by the way.

KristofferC · 2020-10-04T11:30:37Z

src/dump.c

@@ -1124,6 +1124,14 @@ static int64_t write_dependency_list(ios_t *s, jl_array_t **udepsp, jl_array_t *
        }
        write_int32(s, 0); // terminator, for ease of reading

+        // Julia debug mode
+        jl_value_t *julia_debug = (jl_value_t*)jl_get_global(jl_base_module, jl_symbol("julia_debug"));
+        if (julia_debug) {


Perhaps this should call isdebug instead of going for the global?

vchuravy · 2020-10-04T15:22:56Z

Maybe we could use this to turn @assert off in non-debug mode?

KristofferC · 2020-10-04T15:24:44Z

This also disables the assertion macro in normal running code.

tkf · 2020-10-04T17:03:20Z

I think something like this should be implementable with #37595 as a library.

Note that I don't want one to have to add a dependency and do changes in a project file to use debug mode.

Sorry, my wording was not appropriate (edited now). I wasn't against adding this in Base. I just wanted to mention that users now can implement more complex @assert-like macros outside julia.

KristofferC · 2020-10-04T17:18:07Z

I just wanted to mention that users now can implement more complex @assert-like macros outside julia.

Ah, yes. 👍

vchuravy · 2020-10-04T18:12:16Z

base/Base.jl

@@ -66,6 +66,9 @@ time_ns() = ccall(:jl_hrtime, UInt64, ())

 start_base_include = time_ns()

+julia_debug = true


Why not make this a Ref?

thofma · 2020-10-05T15:05:22Z

I want to second what @quinnj said about being super cool and also debugging levels (but those could be built on top I guess).

Apart from that, I think the additional change regarding @assert might be suprising for some people (suprising in the sense of following semver if this is supposed to be included in 1.6, although this is of course subjective). At the moment the docs say that "An assert might be disabled at various optimization levels", but I guess a lot of people expect them to be not removed by default, see for example #10614.

KristofferC · 2020-10-05T17:20:09Z

suprising in the sense of following semver

What do you mean with "following semver"? It is explicitly said in the docs that this check can be omitted and the whole point of adding those docs is to allow for exactly a PR like this.

KristofferC · 2020-10-08T08:08:30Z

The main question here is probably if the current -g option to julia is only for debug mode of Julia itself (separately from package code and thus a bit similar to building clang in debug mode) and there should thus exit a completely separate option to set the debug mode of running julia code (-jg or something) or if it is fine to have these be the same.

vchuravy · 2020-10-14T16:16:35Z

The main question here is probably if the current -g option to julia is only for debug mode of Julia itself (separately from package code and thus a bit similar to building clang in debug mode) and there should thus exit a completely separate option to set the debug mode of running julia code (-jg or something) or if it is fine to have these be the same.

julia-debug is equivalent to building clang in debug mode, and -g is for user code.

KristofferC · 2020-10-14T16:19:22Z

julia-debug is equivalent to building clang in debug mode, and -g is for user code.

Alright. Just to note, we do some compiler checks based on debug_level though, e.g:

julia/base/compiler/ssair/driver.jl

Lines 139 to 141 in 2bd31a0

    
           if JLOptions().debug_level == 2 
        
               @timeit "verify 3" (verify_ir(ir); verify_linetable(ir.linetable)) 
        
           end

vchuravy · 2020-10-14T16:23:11Z

Yeah there is the difference between debug mode for the runtime + C++ parts of the compiler e.g. julia-debug and then the Julia parts of the compiler which use -g. If we want to split that out, I would use -g for user code and introduce a compiler specific flag, but in the end I don't think that difference matters a lot and it is fine to just use -g to mean "all Julia code".

KristofferC · 2020-10-14T16:38:23Z

Okay, and now when I have someone to talk to (yay), what do you think should be done with @assert & co for Base + stdlibs. Just keep it unconditionally enabled for now? To do it "properly" we would need to build and ship two sysimages but I guess we don't think that's worth it.

vchuravy · 2020-10-14T17:11:54Z

what do you think should be done with @assert & co for Base + stdlibs. Just keep it unconditionally enabled for now? To do it "properly" we would need to build and ship two sysimages but I guess we don't think that's worth it.

Yeah we used to ship two systemimages back when we still shipped julia-debug, but I don't it is worth the additional complexity right now.

Of course there are evil tricks one can play with multiversioning, instead of doing this on the Julia level you codegen a call to a intrinsic function and then in when we do multiversioning you can replace that call with a constant and run a round of DCE late in the pipeline. But that suggestion is just me wanting to lure you down to the realm of LLVM ;)

tkf · 2020-11-24T22:24:44Z

Is it hard to replicate what --check-bounds does?

DilumAluthge · 2021-01-02T09:04:20Z

Bump.

@KristofferC Besides the merge conflicts, is there anything else that needs to be done for this?

MasonProtter · 2021-01-15T19:26:20Z

Even though there's always been that warning in the docs, I think there's likely a ton of code out there that's relying on 'improper' uses of @assert just becuase writing @assert cond is more convenient and legible than cond || throw("...").

If we're going to do this, I feel we should at least provide a replacement macro (or method to @assert?) that works like @assert but will throw regardless of whether you're in debug mode.

xref a discussion where this came up

00vareladavid · 2021-04-04T17:41:49Z

I strongly agree with @MasonProtter. Having something which is like @assert but can not be disabled is useful. It seems some people in #25576 are ok with having a @check macro for this purpose.

ericphanson · 2022-01-09T02:54:05Z

#41342 implements an @check macro, by the way. I agree we should provide a simple correct alternative to @assert abuse first before doing something like this PR.

KristofferC added needs news A NEWS entry is required for this change triage This should be discussed on a triage call labels Oct 3, 2020

KristofferC force-pushed the kc/debug_mode branch from 234c881 to d2c1c32 Compare October 3, 2020 17:06

KristofferC mentioned this pull request Oct 3, 2020

RFC: introduce check macro that can be disabled with a flag #25576

Closed

This comment has been minimized.

Sign in to view

tkf reviewed Oct 3, 2020

View reviewed changes

base/loading.jl Outdated Show resolved Hide resolved

KristofferC commented Oct 4, 2020

View reviewed changes

vchuravy reviewed Oct 4, 2020

View reviewed changes

KristofferC mentioned this pull request Oct 7, 2020

@isdefined doesn't work in -g2: Unbound GlobalRef not allowed in value position #37919

Closed

KristofferC force-pushed the kc/debug_mode branch from 67822ee to 1a6c882 Compare October 8, 2020 05:42

KristofferC added needs tests Unit tests are required for this change and removed needs news A NEWS entry is required for this change labels Oct 8, 2020

KristofferC removed the needs tests Unit tests are required for this change label Oct 8, 2020

KristofferC force-pushed the kc/debug_mode branch 4 times, most recently from 1124b5e to 184ab91 Compare October 9, 2020 08:03

disable assert macro unless running in debug mode

ffa738c

KristofferC force-pushed the kc/debug_mode branch from 184ab91 to ffa738c Compare October 9, 2020 13:32

JeffBezanson removed the triage This should be discussed on a triage call label Oct 22, 2020

maleadt mentioned this pull request Jul 6, 2022

WIP/RFC: Add intrinsics for querying the debug level #45948

Draft

KristofferC mentioned this pull request Oct 21, 2022

Allow long-form --debug-info option #47257

Merged

KristofferC mentioned this pull request Oct 17, 2023

Do something about @assert #51729

Open

KristofferC mentioned this pull request Feb 20, 2024

implement a julia specific debug mode with --debug #53404

Open

tgross35 mentioned this pull request Mar 21, 2024

Add @check macro for non-disable-able @assert #41342

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC: implement proper debug mode for packages (with support for re-precompilation) #37874

RFC: implement proper debug mode for packages (with support for re-precompilation) #37874

KristofferC commented Oct 3, 2020 •

edited

Loading

quinnj commented Oct 3, 2020

KristofferC commented Oct 3, 2020 •

edited

Loading

quinnj commented Oct 3, 2020

This comment has been minimized.

tkf commented Oct 3, 2020 •

edited

Loading

KristofferC commented Oct 4, 2020

KristofferC Oct 4, 2020 •

edited

Loading

c42f Oct 5, 2020

KristofferC Oct 5, 2020 •

edited

Loading

c42f Oct 5, 2020

KristofferC Oct 5, 2020

KristofferC Oct 4, 2020

vchuravy commented Oct 4, 2020

KristofferC commented Oct 4, 2020

tkf commented Oct 4, 2020

KristofferC commented Oct 4, 2020

vchuravy Oct 4, 2020

thofma commented Oct 5, 2020

KristofferC commented Oct 5, 2020

KristofferC commented Oct 8, 2020 •

edited

Loading

vchuravy commented Oct 14, 2020

KristofferC commented Oct 14, 2020

vchuravy commented Oct 14, 2020

KristofferC commented Oct 14, 2020

vchuravy commented Oct 14, 2020 •

edited

Loading

tkf commented Nov 24, 2020

DilumAluthge commented Jan 2, 2021

MasonProtter commented Jan 15, 2021 •

edited

Loading

00vareladavid commented Apr 4, 2021

ericphanson commented Jan 9, 2022 •

edited

Loading

		@@ -66,6 +66,9 @@ time_ns() = ccall(:jl_hrtime, UInt64, ())

		start_base_include = time_ns()

		julia_debug = true

RFC: implement proper debug mode for packages (with support for re-precompilation) #37874

Are you sure you want to change the base?

RFC: implement proper debug mode for packages (with support for re-precompilation) #37874

Conversation

KristofferC commented Oct 3, 2020 • edited Loading

quinnj commented Oct 3, 2020

KristofferC commented Oct 3, 2020 • edited Loading

quinnj commented Oct 3, 2020

This comment has been minimized.

tkf commented Oct 3, 2020 • edited Loading

KristofferC commented Oct 4, 2020

KristofferC Oct 4, 2020 • edited Loading

Choose a reason for hiding this comment

c42f Oct 5, 2020

Choose a reason for hiding this comment

KristofferC Oct 5, 2020 • edited Loading

Choose a reason for hiding this comment

c42f Oct 5, 2020

Choose a reason for hiding this comment

KristofferC Oct 5, 2020

Choose a reason for hiding this comment

KristofferC Oct 4, 2020

Choose a reason for hiding this comment

vchuravy commented Oct 4, 2020

KristofferC commented Oct 4, 2020

tkf commented Oct 4, 2020

KristofferC commented Oct 4, 2020

vchuravy Oct 4, 2020

Choose a reason for hiding this comment

thofma commented Oct 5, 2020

KristofferC commented Oct 5, 2020

KristofferC commented Oct 8, 2020 • edited Loading

vchuravy commented Oct 14, 2020

KristofferC commented Oct 14, 2020

vchuravy commented Oct 14, 2020

KristofferC commented Oct 14, 2020

vchuravy commented Oct 14, 2020 • edited Loading

tkf commented Nov 24, 2020

DilumAluthge commented Jan 2, 2021

MasonProtter commented Jan 15, 2021 • edited Loading

00vareladavid commented Apr 4, 2021

ericphanson commented Jan 9, 2022 • edited Loading

KristofferC commented Oct 3, 2020 •

edited

Loading

KristofferC commented Oct 3, 2020 •

edited

Loading

tkf commented Oct 3, 2020 •

edited

Loading

KristofferC Oct 4, 2020 •

edited

Loading

KristofferC Oct 5, 2020 •

edited

Loading

KristofferC commented Oct 8, 2020 •

edited

Loading

vchuravy commented Oct 14, 2020 •

edited

Loading

MasonProtter commented Jan 15, 2021 •

edited

Loading

ericphanson commented Jan 9, 2022 •

edited

Loading