Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Improve performance of parsing Version#3992

Closed
hughbe wants to merge 1 commit intodotnet:masterfrom
hughbe:version-parse
Closed

Improve performance of parsing Version#3992
hughbe wants to merge 1 commit intodotnet:masterfrom
hughbe:version-parse

Conversation

@hughbe
Copy link

@hughbe hughbe commented Mar 30, 2016

Up to x6 perf improvement and x7 allocation reduction by using custom specific parsing code and avoiding split allocations

Benchmark results

benchmark

Benchmark code

Click Here

public static void TimeAction(string prefix, Action action)
{
    var sw = new Stopwatch();
    for (int iter = 0; iter < 5; iter++)
    {
        int gen0 = GC.CollectionCount(0);
        sw.Restart();
        for (int i = 0; i < 10000000; i++)
        {
            action();
        }
        sw.Stop();
        Console.WriteLine($"{prefix}Time: {sw.Elapsed.TotalSeconds}\tGC0: {GC.CollectionCount(0) - gen0}");
    }
}

static void Main(string[] args)
{
    Console.WriteLine("**** Measure x.x ****");
    TimeAction("Old: ", () => new VersionOld("1.1"));
    TimeAction("New: ", () => new VersionNew("1.1"));

    Console.WriteLine("**** Measure x.x.x ****");
    TimeAction("Old: ", () => new VersionOld("1.1.1"));
    TimeAction("New: ", () => new VersionNew("1.1.1"));

    Console.WriteLine("**** Measure x.x.x.x ****");
    TimeAction("Old: ", () => new VersionOld("1.1.1.1"));
    TimeAction("New: ", () => new VersionNew("1.1.1.1"));

    Console.WriteLine("**** Measure x.x.x.x with whitespace ****");
    TimeAction("Old: ", () => new VersionOld("   1   .   1   .   1   .   1   "));
    TimeAction("New: ", () => new VersionNew("   1   .   1   .   1   .   1   "));

    Console.ReadLine();
}

@stephentoub
Copy link
Member

Nice improvement, but wouldn't it be better to improve the performance of Int32.TryParse (at least for the Integer / InvariantCulture case) so that the benefit is available everywhere TryParse is used rather than just for Version? Integer parsing is much more common than version parsing. Or is the improvement somehow specific to Version?

@hughbe
Copy link
Author

hughbe commented Mar 30, 2016

@stephentoub good suggestion, I'll take a look.
The only way that this is specific to Version is the fact that it can't be a negative number.
The code for parsing integers is pretty confusing, but I'll see

Up to x4 perf improvement
@Petermarcu
Copy link
Member

@stephentoub , @hughbe , Any update on this one. Is the current change good as is and the perf improvement in Int32.TryParse is a bonus or should we only be doing it in Int32.TryParse?

@stephentoub
Copy link
Member

I would prefer we not add all of this complexity to Version unless we actually have evidence that Version parsing is a bottleneck somewhere.
cc: @jkotas

@hughbe
Copy link
Author

hughbe commented Oct 12, 2016

I agree. I'll close this!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants