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

error() is a large proportion of our runtime size #6474

Open
steven-johnson opened this issue Dec 6, 2021 · 0 comments
Open

error() is a large proportion of our runtime size #6474

steven-johnson opened this issue Dec 6, 2021 · 0 comments

Comments

@steven-johnson
Copy link
Contributor

steven-johnson commented Dec 6, 2021

While working on #6473, I wanted to see how much code+constdata space we were using for error reporting in our runtime, so I tried making error an alias for SinkPrinter.

Comparing the object size for runtime.o built with host-opencl on OSX, I get:

  • recent master: 180392 bytes
  • with Trim code size for Printer #6473 applied: 164592 bytes (~9.8% smaller)
  • with error() completely nuked via SinkPrinter: 140984 (~22% smaller)

That's a surprisingly large percentage of our runtime. While I don't think runtime size is (currently) a high priority, I suspect this could be reduced substantially by some combination of:

  • Calling existing halide_error_foo() helper functions instead of assembling a bespoke error message
  • Selectively moving some verbose error messages into DEBUG_RUNTIME only
steven-johnson added a commit that referenced this issue Dec 16, 2021
This makes an assumption that ~all of the calls to `error()` in the runtime have descriptive text that is generally only useful for developers, and leaving this text in release builds is of limited-to-no-use. On that assumption:
-Add a new "runtime internal" error code that is a catch-all for these cases
- Add a new `_halide_runtime_error()` error function, which is a smart wrapper that discards all its arguments in non-debug runtimes
- Convert runtime/cuda.cpp as a proof-of-concept of possible code savings. On My OSX box, `bin/host-cuda/runtime.a` is 174128 bytes at current top-of-tree, 163160 bytes with this PR in place. Extending this to the rest of runtime would likely get us down to ~140k if the estimates in #6474 are correct.

Note #1: You could achieve (nearly) the same thing by just changing `error()` to be special-case for DEBUG_RUNTIME, but this formulation is (IMHO) slightly cleaner, since it also allows us to return the error result directly, rather than requiring two statements. It also provides a good excuse to do a once-over of all existing usage, which is probably worthwhile.

- As mentioned above, it basically drops useful text on the floor for release builds, on the assumption that developers can use a debug-runtime build for more details; this may be a terrible assumption. Thoughts?

- This PR makes no attempt to address the really-quite-loose bounds on what can be returned; e.g. there are lots of places we just return a Cuda error where (technically) a halide_error_code_t is expected; this doesn't seem to ever have been a real problem in practice, but it makes my spidey-sense tingle.
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

No branches or pull requests

1 participant