-
Notifications
You must be signed in to change notification settings - Fork 886
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add stacktrace into cudf exception types #13298
Conversation
REALLY good. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies for taking so long to review this. The changes look solid, just a couple of small requests. Can we insert a call to get_stacktrace
in stream_checking_resource_adaptor::verify_stream
? I had originally wanted to extract this function anyway for that purpose since I originally implemented the trace to help find improper stream usage.
/** | ||
* @brief Exception thrown when logical precondition is violated. | ||
* | ||
* This exception should not be thrown directly and is instead thrown by the | ||
* CUDF_EXPECTS macro. | ||
*/ | ||
struct logic_error : public std::logic_error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An alternative would be for each exception type to own a stacktrace_recorder
that is constructed in its constructor. That approach would require ignoring 2 frames instead of just 1, though, and it would be more verbose. I don't have a strong opinion that that is a necessary change here.
Co-authored-by: Vyas Ramasubramani <vyas.ramasubramani@gmail.com>
/merge |
This implements stacktrace and adds a stacktrace string into any exception thrown by cudf. By doing so, the exception carries information about where it originated, allowing the downstream application to trace back with much less effort.
Closes #12422.
Example:
Usage
In order to retrieve a stacktrace with fully human-readable symbols, some compiling options must be adjusted. To make such adjustment convenient and effortless, a new cmake option (
CUDF_BUILD_STACKTRACE_DEBUG
) has been added. Just set this option toON
before building cudf and it will be ready to use.For downstream applications, whenever a cudf-type exception is thrown, it can retrieve the stored stacktrace and do whatever it wants with it. For example:
Follow-up work
The next step would be patching
rmm
to attach stacktrace intormm::
exceptions. Doing so will allow debugging various memory exceptions thrown from libcudf using their stacktrace.Note:
CUDF_BUILD_STACKTRACE_DEBUG
should not be turned on in production as it may affect code optimization. Instead, libcudf compiled with that flag turned on should be used only when needed, when debugging cudf throwing exceptions.-O2
or-O3
, if in Release mode) and replaces by-Og
(optimize for debugging).ON
, the stacktrace will not be available. This is to avoid expensive stracktrace retrieval if the throwing exception is expected.