meta hide_in_stacktrace to hide internal julia functions
#35371
Conversation
|
Test failures seem to be #35356. Rebasing... |
This implements a hide_in_backtrace meta which may be attached to individual methods (via a flag on the CodeInfo) to mark them for hiding in backtraces. Use this infrastructure to hide several julia methods by default (these can still be seen using the internal_funcs flag to backtrace()) * error(), backtrace(), stacktrace() * keyword sorters
This reunifies the versions of include() in MainInclude and Base which have started diverging after being duplicated for improved stack traces. @ hide_in_backtrace provides a solution where the implementation is marked as internal.
Here we introduce an explicit codeinfo_ver version number to track the CodeInfo struct. Core.CodeInfo has seen multiple changes between Julia versions and the addition of the hide_in_stacktrace flag is not possible to detect via the types which exist in the serialized stream. To solve this problem and to avoid backward compatibility code becoming more convoluted and hard to maintain in the future, it seems simplest to introduce an Int32 version flag which should be a single byte of overhead. An alternative which was considered was to rely on the value of the header ser_version. However, this would need to be somehow stored with any user-defined subtype of AbstractSerializer which would be a API breaking change.
c42f
force-pushed
the
cjf/hide-in-stacktrace-meta
branch
from
fcd09ec
to
4aef920
Compare
|
I would argue that the default heuristic should probably be changed. Stack traces should, by default, only show functions that are exported. Yes, that means we lose information in bug reports, which is visible to people here and therefore will be unpopular, but the flip side is that we are scaring huge numbers of people away from Julia with long, unintelligible stack traces, which is a silent but more worse problem. |
You mean literally |
|
We probably should have a way to declare something to be public without exporting it, but we don't so for now |
|
I really disagree with removing a lot of frames from stack traces. When the truth is ugly, lying about it tends not to make things better. Somebody who doesn't want to look at a stack trace will still not want to look at it, but if somebody does want to try to trace through and see what happened, it will become impossible. Stack traces always and inherently show implementation details. That's their purpose. The fact that If we want to do something here, rather than having a heuristic that leaves out random frames we should address this with formatting or UI. For example maybe show only the top 1 or 2 frames with a key you can press to see the rest. Though that will certainly reduce the usefulness of bug reports, as you noted. |
|
The inlining pass can see this flag, so it should be possible to have it remove the frame info for a function it's inlining instead of implying noinline. |
|
I think you're really viewing stack traces purely from the point of view of a developer of the system, not as a user of the system. For most users, the primary purpose of a stack trace is to communicate to them where they did something wrong, not where in the guts of the system the thing they did wrong happened to trigger a problem. I agree that it is a UI problem, however, and we don't need to not capture those stack frames, we just need to not show them all the time. |
|
I understand that seeing a large stack trace full of internals is confusing and unhelpful. That's not hard to imagine. I just think there is a large asymmetry: hiding information could be a big nuisance for a (julia or package) developer or anybody who might want to look into the stack trace, but is at best a small help to somebody who finds stack traces confusing. I agree it would be great if we could just show what the user did wrong, but I don't think that's possible. For example, if you pass an invalid value to a function that doesn't have an explicit check for that case, human knowledge is required to know at which level the invalid value arose. It also might just be a bug in the library code. But I do think we can and should improve stack trace ergonomics (e.g. truncating huge types, formatting, other things discussed in #33065), and I'm open to UI changes. |
This is exactly what this is intended to implement: a way to refine the existing formatting heuristic that C frames are left out. So perhaps a compromise would be to not use it in Probably a better option would be to open a log file for each julia session and to log much more detailed backtraces in there, separately from the summary that the user sees by default. Then get people used to sending in their log files, and also have some REPL tooling to see a more full backtrace for the last error?
I considered this but I didn't pursue it yet because I worried it would imply loosing those frames completely in the raw |
|
I agree stack traces need improvements, but removing non exported functions feels much too blunt. Hiding types (like juno does) or having an array display of traces would be a better way to help this. |
|
Here's a summary of a fairly long and productive discussion I had about this with Jeff and Keno. I think Jeff had a very strong intuitive reaction that completely hiding frames is a bad idea... somehow that it's aberrant behavior / against the spirit of what a backtrace should offer / and that offering it to libraries at the Julia level would just be a very bad idea. I didn't buy all the arguments for this, but I'm convinced there was some underlying kernel of truth, if only we could express it clearly. Eventually we got to discussing why some frames may be considered truly internal which justifies hiding them in the current implementation, and why most frames do not qualify for this treatment. Here's my takeaways from that discussion: Truly internal frames are those which exist only as part of the "underlying computational substrate" which implements julia semantics. Personally I believe examples of this are:
Frames which should be user visible are any frames which would logically be part of the call stack if there was a formal notion of the abstract machine on which Julia runs. These include
Now we see how our current heuristics do fail us in certain circumstances:
Conversely I think the following comment in debuginfo is another interesting case where we correctly distinguish between internal vs user-visible frames: I hope that categorization is not too controversial and can help frame the discussion here? Unfortunately things are never entirely clear cut. For example, if I move all the implementation detail from an internal C function like |
This is relative to who you ask for the stacktrace --- if you ask the underlying julia runtime, you should get all the JuliaInterpreter guts, but if you ask JuliaInterpreter you should get the stack trace of its subject program. I'll add that the other reason we have decided to hide frames in the past is if they would be part of every stack trace. For example if every REPL input begins evaluation with the same couple frames, the REPL should probably not show those. But I think removing various frames in the middle of a stack trace is a different story. |
Sure, completely agreed. It's not so much that the internal heuristics fail us, but that But rather than nitpicking at this one small part of the summary, what do you think about the point that there are some truly internal frames? Yesterday you argued that you never want to see the C frames from the runtime but allowed that C frames from external libs can be relevant to the user. I think the distinction comes down to "Which frames exist only as an implementation detail of how the abstract julia call stack is concretely realized in the runtime?" |
Hrm, I don't entirely like how I wrote that. To be quite clear, I'm not unhappy about nitpicking but I want to focus on the larger point: we manifestly do have hidden frames (currently all of the C frames and a few ad-hoc exceptions to make Julia frames hidden). So can we agree on some definition for what an internal frame actually is? |
Yes I agree. |
|
Cool, so under that definition of "internal" things become clearer. If we restrict to discussing a possible
I could go either way on Perhaps that sounds iffy, but there's no slippery slope here. The same argument can not be extended to the rest of We can have a separate discussion about UI hints. One small point: users will not be tempted to abuse |
|
For anyone who happens to be following with interest — we're deferring this for now to avoid getting too bogged down with difficult design work. There's bigger usability gains to be made elsewhere. |
|
Rust elides some frames from the start and end of backtraces by default. (Mentioned in https://blog.rust-lang.org/2020/10/08/Rust-1.47.html#shorter-backtraces.) Some implementation details:
|
|
So the equivalent for us would be to elide frames 3-6 in in |
|
Right, it's not very complicated. Just a slightly more general way to do what the REPL is already doing. |
|
Just linking this PR to my |
Currently
stacktraceuses the heuristic that every C function is internal, and every Julia function is of interest to the user. This works surprisingly well, but on occasion it can lead to confusion or redundancy. A particular example wasinclude()as noted in #33065. This has since been worked around, but at the cost of duplicating the implementation ofinclude().This PR implements a
hide_in_stacktracemeta to indicate that certain frames should be hidden, reviving an idea originally mentioned in #33065 (comment). (Though with the motivation of removing the duplication ofincludein preparation for moving more of the runtime C frontend code into Julia code as part of #35243)I've used this to mark a few functions as hidden:
errorinclude()internalsMinor caveats and yak shaving
@hide_in_stacktraceimplynoinlinefor now. This seems ok for the types of backtrace-producing and error throwing functions this is typically useful for.CodeInfofrom older Julia versions, but any added CodeInfo flags areBoolwhich leaves us unable to infer the CodeInfo version. To fix this, I've added an explicit codeinfo version to the serialized form. The rationale for this is explained in much more detail in the third commit message.