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

Investigate perf improvements for the built-in JsonNamingPolicy (SnakeCase) for DictionaryKeyPolicy #31486

Closed
ahsonkhan opened this issue Nov 13, 2019 · 7 comments
Labels
area-System.Text.Json backlog-cleanup-candidate An inactive issue that has been marked for automated closure. help wanted [up-for-grabs] Good issue for external contributors tenet-performance Performance related issue
Milestone

Comments

@ahsonkhan
Copy link
Member

One option to reduce the allocations could be to use ValueStringBuilder (see dotnet/corefx#41354 (comment)), though that doesn't help the throughput too much. It might be possible to re-implement the algorithm in ConvertName to be faster. Another idea might be to special case the first character to simplify some of the conditions within the loop (like currentIndex > 0).

BenchmarkDotNet=v0.11.5.1159-nightly, OS=Windows 10.0.18362
Intel Core i7-6700 CPU 3.40GHz (Skylake), 1 CPU, 8 logical and 4 physical cores
.NET Core SDK=3.1.100-preview1-014459
  [Host]     : .NET Core 3.0.0 (CoreCLR 4.700.19.46205, CoreFX 4.700.19.46214), X64 RyuJIT
  Job-HPZZRZ : .NET Core 3.0.0 (CoreCLR 4.700.19.46205, CoreFX 4.700.19.46214), X64 RyuJIT

PowerPlanMode=00000000-0000-0000-0000-000000000000  IterationTime=250.0000 ms  MaxIterationCount=10  
MinIterationCount=5  WarmupCount=1  
Method Mean Error StdDev Median Min Max Ratio RatioSD Gen 0 Gen 1 Gen 2 Allocated
Default 2.426 us 0.0473 us 0.0281 us 2.425 us 2.391 us 2.473 us 1.00 0.00 0.1567 - - 656 B
Camel 3.040 us 0.0895 us 0.0592 us 3.027 us 2.979 us 3.150 us 1.26 0.04 0.2491 - - 1048 B
Snake 4.293 us 0.0939 us 0.0621 us 4.280 us 4.195 us 4.397 us 1.77 0.01 0.5624 - - 2432 B
public class SerializeDictionaryWithNamingPolicy
{
    JsonSerializerOptions _camelOptions;
    JsonSerializerOptions _snakeOptions;

    Dictionary<string, string> _foo;

    [GlobalSetup]
    public void Setup()
    {
        _camelOptions = new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase, DictionaryKeyPolicy = JsonNamingPolicy.CamelCase };
        _snakeOptions = new JsonSerializerOptions { PropertyNamingPolicy = new JsonSnakeCaseNamingPolicy(), DictionaryKeyPolicy = new JsonSnakeCaseNamingPolicy() };

        _foo = new Dictionary<string, string>()
        {
            ["ID"] = "A",
            ["url"] = "A",
            ["URL"] = "A",
            ["THIS IS SPARTA"] = "A",
            ["IsCIA"] = "A",
            ["iPhone"] = "A",
            ["IPhone"] = "A",
            ["xml2json"] = "A",
            ["already_snake_case"] = "A",
            ["IsJSONProperty"] = "A",
            ["ABCDEFGHIJKLMNOP"] = "A",
            ["Hi!! This is text.Time to test."] = "A",
        };
    }

    [BenchmarkCategory(Categories.CoreFX, Categories.JSON)]
    [Benchmark(Baseline = true)]
    public string Default()
    {
        return JsonSerializer.Serialize(_foo);
    }

    [BenchmarkCategory(Categories.CoreFX, Categories.JSON)]
    [Benchmark]
    public string Camel()
    {
        return JsonSerializer.Serialize(_foo, _camelOptions);
    }

    [BenchmarkCategory(Categories.CoreFX, Categories.JSON)]
    [Benchmark]
    public string Snake()
    {
        return JsonSerializer.Serialize(_foo, _snakeOptions);
    }
}
@YohDeadfall
Copy link
Contributor

Out of curiosity, could you run benchmarks only for ConvertName methods?

@ahsonkhan
Copy link
Member Author

I did, previously :)

public class ConvertNamePerfComparison
{
    JsonNamingPolicy _camelPolicy;
    JsonSnakeCaseNamingPolicy _snakePolicy;

    [Params("ID", "url", "URL", "THIS IS SPARTA", "IsCIA", "iPhone", "IPhone", "xml2json", "already_snake_case", "Hi!! This is text. Time to test.", "IsJSONProperty", "ABCDEFGHIJKLMNOP")]
    public string InputString { get; set; }

    [GlobalSetup]
    public void Setup()
    {
        _camelPolicy = JsonNamingPolicy.CamelCase;
        _snakePolicy = new JsonSnakeCaseNamingPolicy();
    }

    [BenchmarkCategory(Categories.CoreFX, Categories.JSON)]
    [Benchmark (Baseline =true)]
    public string Camel()
    {
        return _camelPolicy.ConvertName(InputString);
    }

    [BenchmarkCategory(Categories.CoreFX, Categories.JSON)]
    [Benchmark]
    public string Snake()
    {
        return _snakePolicy.ConvertName(InputString);
    }
}
BenchmarkDotNet=v0.11.5.1159-nightly, OS=Windows 10.0.18362
Intel Core i7-6700 CPU 3.40GHz (Skylake), 1 CPU, 8 logical and 4 physical cores
.NET Core SDK=3.1.100-preview1-014459
  [Host]     : .NET Core 3.0.0 (CoreCLR 4.700.19.46205, CoreFX 4.700.19.46214), X64 RyuJIT
  Job-PHLPOC : .NET Core 3.0.0 (CoreCLR 4.700.19.46205, CoreFX 4.700.19.46214), X64 RyuJIT

PowerPlanMode=00000000-0000-0000-0000-000000000000  IterationTime=250.0000 ms  MaxIterationCount=10  
MinIterationCount=5  WarmupCount=1  
Method InputString Mean Error StdDev Median Min Max Ratio RatioSD Gen 0 Gen 1 Gen 2 Allocated
Camel ABCDEFGHIJKLMNOP 144.050 ns 2.3565 ns 1.0463 ns 144.221 ns 142.251 ns 145.340 ns 1.00 0.00 0.0130 - - 56 B
Snake ABCDEFGHIJKLMNOP 286.472 ns 11.2365 ns 7.4322 ns 284.014 ns 274.514 ns 298.664 ns 1.97 0.04 0.0395 - - 168 B
Camel Hi!! (...)test. [32] 26.888 ns 1.0042 ns 0.6642 ns 26.523 ns 26.255 ns 27.806 ns 1.00 0.00 0.0210 - - 88 B
Snake Hi!! (...)test. [32] 206.047 ns 5.6577 ns 3.7422 ns 205.644 ns 200.347 ns 211.571 ns 7.67 0.25 0.0548 - - 232 B
Camel ID 29.211 ns 0.7578 ns 0.4509 ns 29.159 ns 28.243 ns 29.929 ns 1.00 0.00 0.0076 - - 32 B
Snake ID 53.312 ns 1.8611 ns 1.1075 ns 53.397 ns 51.168 ns 55.343 ns 1.83 0.04 0.0268 - - 112 B
Camel IPhone 25.988 ns 0.6220 ns 0.3702 ns 25.841 ns 25.507 ns 26.549 ns 1.00 0.00 0.0095 - - 40 B
Snake IPhone 69.932 ns 1.8686 ns 1.1120 ns 69.844 ns 68.476 ns 71.354 ns 2.69 0.07 0.0305 - - 128 B
Camel IsCIA 22.477 ns 0.4576 ns 0.2723 ns 22.545 ns 22.139 ns 23.023 ns 1.00 0.00 0.0076 - - 32 B
Snake IsCIA 87.861 ns 3.0060 ns 0.7807 ns 87.468 ns 87.330 ns 89.208 ns 3.92 0.07 0.0303 - - 128 B
Camel IsJSONProperty 23.641 ns 0.8762 ns 0.5795 ns 23.619 ns 22.858 ns 24.567 ns 1.00 0.00 0.0134 - - 56 B
Snake IsJSONProperty 182.701 ns 11.7031 ns 6.9644 ns 180.305 ns 173.343 ns 192.113 ns 7.71 0.38 0.0376 - - 160 B
Camel THIS IS SPARTA 48.576 ns 2.4179 ns 1.4389 ns 48.879 ns 45.882 ns 50.816 ns 1.00 0.00 0.0132 - - 56 B
Snake THIS IS SPARTA 270.268 ns 27.4163 ns 16.3150 ns 265.223 ns 252.873 ns 305.450 ns 5.57 0.34 0.0375 - - 160 B
Camel URL 37.835 ns 1.3932 ns 0.8291 ns 38.104 ns 36.627 ns 39.059 ns 1.00 0.00 0.0076 - - 32 B
Snake URL 84.754 ns 14.3589 ns 9.4976 ns 82.785 ns 74.739 ns 99.698 ns 2.24 0.26 0.0286 - - 120 B
Camel already_snake_case 2.275 ns 0.5777 ns 0.3821 ns 2.380 ns 1.801 ns 2.884 ns 1.00 0.00 - - - -
Snake already_snake_case 107.395 ns 4.0254 ns 2.3954 ns 107.700 ns 103.499 ns 110.372 ns 47.17 7.44 0.0436 - - 184 B
Camel iPhone 2.073 ns 0.1440 ns 0.0953 ns 2.067 ns 1.907 ns 2.219 ns 1.00 0.00 - - - -
Snake iPhone 59.832 ns 1.0612 ns 0.2756 ns 59.790 ns 59.472 ns 60.177 ns 28.28 0.72 0.0304 - - 128 B
Camel url 2.262 ns 0.1329 ns 0.0791 ns 2.217 ns 2.181 ns 2.382 ns 1.00 0.00 - - - -
Snake url 36.696 ns 1.3405 ns 0.8866 ns 36.580 ns 35.202 ns 38.168 ns 16.24 0.52 0.0286 - - 120 B
Camel xml2json 2.387 ns 0.3469 ns 0.2295 ns 2.370 ns 2.021 ns 2.730 ns 1.00 0.00 - - - -
Snake xml2json 61.309 ns 3.3973 ns 2.2471 ns 60.858 ns 58.979 ns 66.335 ns 25.96 3.35 0.0323 - - 136 B

@YohDeadfall
Copy link
Contributor

Thanks, will take a look at it, but only starting next week.

@YohDeadfall
Copy link
Contributor

@ahsonkhan Probably it could be made faster by eliminating nullable types which I introduced to clarify the logic.

@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@msftgits msftgits added this to the 5.0 milestone Feb 1, 2020
@layomia layomia added the help wanted [up-for-grabs] Good issue for external contributors label Apr 14, 2020
@layomia layomia modified the milestones: 5.0.0, Future Jun 20, 2020
@iSazonov
Copy link
Contributor

I wonder - dotnet/corefx#41354 is not merged (Although a lot of effort has been spent there
!) but the issue is a open :-)

@ghost
Copy link

ghost commented Oct 17, 2021

Due to lack of recent activity, this issue has been marked as a candidate for backlog cleanup. It will be closed if no further activity occurs within 14 more days. Any new comment (by anyone, not necessarily the author) will undo this process.

This process is part of the experimental issue cleanup initiative we are currently trialing in a limited number of areas. Please share any feedback you might have in the linked issue.

@ghost
Copy link

ghost commented Nov 5, 2021

This issue will now be closed since it had been marked no recent activity but received no further activity in the past 14 days. It is still possible to reopen or comment on the issue, but please note that the issue will be locked if it remains inactive for another 30 days.

@ghost ghost closed this as completed Nov 5, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Dec 5, 2021
@eiriktsarpalis eiriktsarpalis added the backlog-cleanup-candidate An inactive issue that has been marked for automated closure. label Feb 18, 2022
@ghost ghost removed the no-recent-activity label Feb 18, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.Text.Json backlog-cleanup-candidate An inactive issue that has been marked for automated closure. help wanted [up-for-grabs] Good issue for external contributors tenet-performance Performance related issue
Projects
None yet
Development

No branches or pull requests

6 participants