Use simple array for AggregateException inner exceptions storage #44787
Conversation
|
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
src/libraries/System.Private.CoreLib/src/System/AggregateException.cs
Outdated
Show resolved
Hide resolved
| // because we're going to put them into a ReadOnlyCollection which simply reuses | ||
| // the list passed in to it. We don't want callers subsequently mutating. | ||
| Exception[] exceptionsCopy = new Exception[innerExceptions.Count]; | ||
| _innerExceptions = cloneExceptions ? new Exception[innerExceptions.Length] : innerExceptions; |
There was a problem hiding this comment.
(Exception[])innerExceptions.Clone()?
There was a problem hiding this comment.
Is it better with the cast from object?
There was a problem hiding this comment.
Sorry, not sure what you mean by that question.
There was a problem hiding this comment.
I meant is Clone version better if it has to generate a cast check?
|
What is the purpose of this change? |
@stephentoub remove the unnecessary wrapping and ROS dependency and interfaces casts. |
|
But it still uses ROC: What am i missing? |
|
It's used from a single property now which is rarely used (and could be avoided if needed). The linker will be able to remove the field by default whereas before it was a dependency from ctor which made it essentially non-trimmable. |
Not really. Most code paths that explicitly catch AggregateException then use InnerExceptions. And even this PR is insufficient I'd expect to be able to ever trim it from corelib, since Task itself relies on InnerExceptions in a way that's unlikely to be trimmable. If the goal is to enable ROC to be trimmable in most apps, let's make sure this PR actually does that. |
This didn't show up for wasm but I fixed the calls anyway to make this change more effective. |
src/libraries/System.Private.CoreLib/src/System/AggregateException.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Private.CoreLib/src/System/AggregateException.cs
Outdated
Show resolved
Hide resolved
…tion.cs Co-authored-by: Stephen Toub <[email protected]>
ghost
locked as resolved and limited conversation to collaborators
to drop unnecessary wrapping into ReadOnlyCollection. The change also simplified ctor chaining to avoid extra wrapping/casts where it's not needed.