Skip to content
This repository has been archived by the owner on Dec 14, 2018. It is now read-only.

Waste less memory when model binding #5499

Closed
dougbu opened this issue Nov 3, 2016 · 4 comments
Closed

Waste less memory when model binding #5499

dougbu opened this issue Nov 3, 2016 · 4 comments
Assignees

Comments

@dougbu
Copy link
Member

dougbu commented Nov 3, 2016

Loops through ModelMetadata.Properties in ComplexTypeModelBinder eat 1.3% of the total memory used in the BigModelBinding scenario. Boxing the List<ModelMetadata> Enumerators could be completely avoided by switching from foreach to for (twice) in this class.

Similarly, enumerating children of the DefaultComplexObjectValidationStrategy eats about 0.8% of the total used in the same scenario. The fix here is a bit more complicated because that Enumerator class is not currently exposed. Still, it could be exposed and made into a struct because everything's in an Internal namespace.

BigModelBinding

Exercises HomeController.Index() view using ./loadtest.ps1. Binds an instance of the Model using form data from ./postdata.txt.

Excess Enumerators

bigmodelbinding - baseline

@rynowak
Copy link
Member

rynowak commented Nov 4, 2016

I don't believe you that the fix with DefaultComplexObjectValidationStrategy is so straightforward. There's an interface involved.

@dougbu
Copy link
Member Author

dougbu commented Nov 4, 2016

I don't believe you that the fix with DefaultComplexObjectValidationStrategy is so straightforward. There's an interface involved.

@rynowak agreed. I said that one was (at least) "a bit more complicated". Would likely require special-casing that strategy class or (somehow) reducing the size of its Enumerator class.

@Eilon Eilon added this to the 1.2.0 milestone Nov 23, 2016
@Eilon
Copy link
Member

Eilon commented Nov 23, 2016

Let's do the simple change for avoiding enumerator allocations. Let's not do the rest of the change, as it is unlikely to simple, and the benefits are small.

dougbu added a commit that referenced this issue Nov 25, 2016
- #5499
- switch `foreach` to `for` and use less Linq when accessing `modelMetadata.Properties`
dougbu added a commit that referenced this issue Dec 6, 2016
- #5499
- switch `foreach` to `for` and use less Linq when accessing `modelMetadata.Properties`
dougbu added a commit that referenced this issue Dec 6, 2016
- #5499
- switch `foreach` to `for` and use less Linq when accessing `modelMetadata.Properties`
- change backing field for `ModelExplorer.Properties` from a list to an array
@dougbu
Copy link
Member Author

dougbu commented Dec 6, 2016

7178464

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants