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

.net 7 cycles Performance slow #78089

Closed
quan0zhou opened this issue Nov 9, 2022 · 5 comments · Fixed by #78130
Closed

.net 7 cycles Performance slow #78089

quan0zhou opened this issue Nov 9, 2022 · 5 comments · Fixed by #78130
Labels

Comments

@quan0zhou
Copy link

Description

image

    public class BenchmarkSum
    {
        private JsonObject[] _jsonObjs;


        [GlobalSetup]
        public void Setup()
        {
            _jsonObjs = new JsonObject[100000];
        }

        [Benchmark]
        public void Sum()
        {
            for (int i = 0; i < 100000; i++)
            {
                _jsonObjs[i] = new JsonObject()
                {
                    ["aaa"] = 1,
                    ["bbbb"] = 2,
                    ["cccc"] = new JsonArray()
                    {
                        new JsonObject()
                        {
                            ["aaaa"] = "aaaaa",
                            ["ffffff"] = "ffffffffffffff"
                        }
                    }
                };
            }
        }
    }

Configuration

Regression?

Data

Analysis

@quan0zhou quan0zhou added the tenet-performance Performance related issue label Nov 9, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@stephentoub
Copy link
Member

@eiriktsarpalis, @krwq, here's an allocation trace with the repro but for only 1,000 iterations instead of 100,000 iterations. On the left is .NET 6, on the right is .NET 7:
image
That's a lot of additional allocation. Is this expected?

@eiriktsarpalis eiriktsarpalis added the untriaged New issue has not been triaged by the area owner label Nov 9, 2022
@eiriktsarpalis
Copy link
Member

Superficially, this appears to only be creating DOM instances without performing any serializations, so I'm surprised that any JsonSerializerOptions instance are materialized. Would need to take a closer look.

@ghost
Copy link

ghost commented Nov 9, 2022

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

image

    public class BenchmarkSum
    {
        private JsonObject[] _jsonObjs;


        [GlobalSetup]
        public void Setup()
        {
            _jsonObjs = new JsonObject[100000];
        }

        [Benchmark]
        public void Sum()
        {
            for (int i = 0; i < 100000; i++)
            {
                _jsonObjs[i] = new JsonObject()
                {
                    ["aaa"] = 1,
                    ["bbbb"] = 2,
                    ["cccc"] = new JsonArray()
                    {
                        new JsonObject()
                        {
                            ["aaaa"] = "aaaaa",
                            ["ffffff"] = "ffffffffffffff"
                        }
                    }
                };
            }
        }
    }

Configuration

Regression?

Data

Analysis

Author: quan0zhou
Assignees: -
Labels:

area-System.Text.Json, tenet-performance

Milestone: -

@stephentoub
Copy link
Member

@eiriktsarpalis, I assume the intent was for this to be static?!

private protected readonly JsonSerializerOptions s_defaultOptions = new();

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Nov 9, 2022
@ghost ghost removed in-pr There is an active PR which will close this issue when it is merged untriaged New issue has not been triaged by the area owner labels Nov 10, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants