cdisselkoen
commented
4 months ago See discussion in #928. We will keep this issue on the backlog for now, as it represents a pretty large effort for an unclear benefit. But, if in the future we are looking for ways to improve performance and identify this as relevant to performance metrics we care about, we can revisit this issue.
Describe the improvement you'd like to request
Our code uses
Result<T, SomeErr>a lot, both internally and in its public APIs. In many cases theseSomeErrtypes are large enums, large in the physical sense (many bytes) due to information stored in each error variant. For one example, Core'sEntitiesErroris 376 bytes; a singleEntityUIDis 120 bytes. This means that our Result types are very large as well, which requires large amounts of stack space be allocated and de-allocated as we call functions, and also perhaps hurts our CPU cache performance. (And it causes Clippy warnings in some cases.)As a proposed general solution for this, we should transition from
Result<T, SomeErr>toResult<T, Box<SomeErr>>everywhere -- at least in internal APIs, but possibly in our public APIs as well, while we're already overhauling error types for #745. BecauseBoxis small (conceptually just a pointer),Result<T, Box<SomeErr>>will have size very close to the size ofT. If this has noticeable performance implications at all, it will likely be to improve the performance of the happy-path (due to smaller Result types) at the expense of some performance on the error path (due to more heap allocation and pointer-chasing), which is an acceptable/desirable tradeoff.Describe alternatives you've considered
Boxing the types in each variant; for instance
EntitiesError::SomeVariantName(Box<SomeErr>). This is more verbose (requires us writingBoxmore places) and also impedes matching on the error more than the proposed solution, I think.Additional context
No response
Is this something that you'd be interested in working on?