Skip to content
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

Move error handling to DALI core. #867

Merged
merged 4 commits into from May 14, 2019
Merged

Move error handling to DALI core. #867

merged 4 commits into from May 14, 2019

Conversation

mzient
Copy link
Contributor

@mzient mzient commented May 10, 2019

  • Move error handling primitives to DALI core.
  • Remove control-by-exception in DisplacementFilter
  • Handle all std::exceptions, not just std::runtime_errors
  • Fix hang when throwing something other than std::exception
  • Add dynlink_cuda target to facilitate dynamic linking to libcuda from all executable targets (.so and tests)

@mzient
Copy link
Contributor Author

mzient commented May 10, 2019

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [731817]: BUILD STARTED


namespace dali {

class DeviceGuard {
public:
DeviceGuard() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the point of this?

Copy link
Contributor Author

@mzient mzient May 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DeviceGuard without parameter just saves and restores current device, for the sake of subsequent changes, e.g. iterating over all devices and setting them with cudaSetDevice.

#define FILE_AND_LINE __FILE__ ":" DALI_STR(__LINE__)

#define DALI_MESSAGE(str)\
(std::string("[" FILE_AND_LINE "] ") + str + dali::GetStacktrace())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering: is this creating several temporary strings or just one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It creates several, but with move semantics it might still be as efficient as +=. The question is: is it?

(std::string("[" FILE_AND_LINE "] ") + str + dali::GetStacktrace())

#define DALI_FAIL(str) \
throw dali::DALIException(DALI_MESSAGE(str)); \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could be a function

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope. FILE and LINE will lose their meaning.

dali/pipeline/executor/executor.h Show resolved Hide resolved
@dali-automaton
Copy link
Collaborator

CI MESSAGE: [731817]: BUILD PASSED

Signed-off-by: Michal Zientkiewicz <michalz@nvidia.com>
Signed-off-by: Michal Zientkiewicz <michalz@nvidia.com>
Move dynlink_cuda to core.
Add dynlink_cuda static library.

Signed-off-by: Michal Zientkiewicz <michalz@nvidia.com>
@mzient
Copy link
Contributor Author

mzient commented May 13, 2019

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [733748]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [733748]: BUILD PASSED

Signed-off-by: Michal Zientkiewicz <michalz@nvidia.com>
@mzient mzient requested a review from klecki May 14, 2019 08:34
@mzient
Copy link
Contributor Author

mzient commented May 14, 2019

!build

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [736675]: BUILD STARTED

@dali-automaton
Copy link
Collaborator

CI MESSAGE: [736675]: BUILD PASSED

@mzient mzient merged commit bcbe7a6 into NVIDIA:master May 14, 2019
haoxintong pushed a commit to haoxintong/DALI that referenced this pull request Jul 16, 2019
* Move error_handling and cuda_utils to dali_core.
* Fix hang when something other than `std::exception` is thrown.
* Add CUDA error handling.
* Move dynlink_cuda to core.
* Add dynlink_cuda static library.

Signed-off-by: Michal Zientkiewicz <michalz@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants