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

Improved compatibility of Format String #30

Merged
merged 6 commits into from
Jan 8, 2021

Conversation

udaken
Copy link
Contributor

@udaken udaken commented Aug 7, 2020

BCL-Compatibility

The format of ZString has some incompatibility with BCL.
The main missing piece is the "Alignment Component".
There are also differences in the handling of whitespace and incorrect formatting.

Some of the cases are listed below.

Format String String.Format ZString.Format
Test("{0,10:X}{{0}}{1,-10:c}", Guid.NewGuid(), DateTime.Now.TimeOfDay.Negate()); Accept throws FormatException
Test("{1,-1}{0,1}", Int64.MinValue, Int64.MaxValue); "9223372036854775807-9223372036854775808" "-1-1"
Test("{00 , 01 }, {01 ,02 }, {2 ,3 :D }, {3 ,4: X }", 2, 3, 5, 7) Accept throws FormatException
Test("}") throws FormatException throws IndexOutOfRangeException
Test("{ 0,0}, 9999") throws FormatException "9999"

I have added a test in CompositeFormatTest.cs that you can check out.

Performance

I've refactored the formatting parsing process.
As a side effect, it makes most of the parsing process about 20% faster. (compared to ver 2.2.0)

However, in the case of short format string (like x:{0}, y:{1}), Utf16PreparedFormat and Utf8PreparedFormat are about 20% slower.
I have optimized the PreparedFormat class, but Utf16PreparedFormat is still slow.

Format is acceptable because the impact is limited.

FormatBenchmark.cs

Method FormatString Mean Error StdDev Ratio RatioSD Code Size Gen 0 Gen 1 Gen 2 Allocated
Format_ This (...)orld. [62] 208.25 ns 2.284 ns 2.136 ns 1.00 0.00 3147 B 0.0181 - - 152 B
FormatN This (...)orld. [62] 166.63 ns 2.186 ns 2.045 ns 0.80 0.01 4172 B 0.0181 - - 152 B
CreateUtf16PreparedFormat_ This (...)orld. [62] 303.47 ns 4.553 ns 4.259 ns 1.00 0.00 2796 B 0.0820 - - 688 B
CreateUtf16PreparedFormatN This (...)orld. [62] 297.62 ns 5.283 ns 4.942 ns 0.98 0.01 2030 B 0.0658 - - 552 B
CreateUtf8PreparedFormat_ This (...)orld. [62] 300.89 ns 6.018 ns 5.629 ns 1.00 0.00 2799 B 0.0820 - - 688 B
CreateUtf8PreparedFormatN This (...)orld. [62] 338.32 ns 5.217 ns 4.880 ns 1.12 0.03 3430 B 0.0849 - - 712 B
Utf16PreparedFormat_ This (...)orld. [62] 79.30 ns 1.162 ns 1.087 ns 1.00 0.00 2597 B 0.0181 - - 152 B
Utf16PreparedFormatN This (...)orld. [62] 96.11 ns 1.342 ns 1.255 ns 1.21 0.02 2188 B 0.0181 - - 152 B
Utf8PreparedFormat_ This (...)orld. [62] 167.63 ns 2.481 ns 2.321 ns 1.00 0.00 2569 B 0.0181 - - 152 B
Utf8PreparedFormatN This (...)orld. [62] 137.85 ns 2.213 ns 2.070 ns 0.82 0.02 2356 B 0.0181 - - 152 B
Utf16StringBuilderAppendFormat_ This (...)orld. [62] 216.73 ns 0.727 ns 0.680 ns 1.00 0.00 7425 B - - - -
Utf16StringBuilderAppendFormatN This (...)orld. [62] 162.42 ns 1.610 ns 1.506 ns 0.75 0.01 7393 B - - - -
Utf8StringBuilderAppendFormat_ This (...)orld. [62] 245.23 ns 0.942 ns 0.835 ns 1.00 0.00 7856 B - - - -
Utf8StringBuilderAppendFormatN This (...)orld. [62] 229.33 ns 0.944 ns 0.883 ns 0.93 0.00 10714 B - - - -
Format_ x:{0}, y:{1} 133.78 ns 1.151 ns 1.076 ns 1.00 0.00 3147 B 0.0057 - - 48 B
FormatN x:{0}, y:{1} 99.33 ns 1.070 ns 1.000 ns 0.74 0.01 4172 B 0.0057 - - 48 B
CreateUtf16PreparedFormat_ x:{0}, y:{1} 206.94 ns 2.647 ns 2.476 ns 1.00 0.00 2796 B 0.0448 - - 376 B
CreateUtf16PreparedFormatN x:{0}, y:{1} 126.35 ns 2.426 ns 2.269 ns 0.61 0.01 2030 B 0.0372 - - 312 B
CreateUtf8PreparedFormat_ x:{0}, y:{1} 203.51 ns 1.199 ns 1.122 ns 1.00 0.00 2799 B 0.0448 - - 376 B
CreateUtf8PreparedFormatN x:{0}, y:{1} 169.32 ns 2.718 ns 2.542 ns 0.83 0.01 3430 B 0.0420 - - 352 B
Utf16PreparedFormat_ x:{0}, y:{1} 69.21 ns 0.898 ns 0.840 ns 1.00 0.00 2597 B 0.0057 - - 48 B
Utf16PreparedFormatN x:{0}, y:{1} 81.91 ns 1.049 ns 0.982 ns 1.18 0.02 2188 B 0.0057 - - 48 B
Utf8PreparedFormat_ x:{0}, y:{1} 126.13 ns 0.971 ns 0.860 ns 1.00 0.00 2569 B 0.0057 - - 48 B
Utf8PreparedFormatN x:{0}, y:{1} 111.22 ns 0.655 ns 0.613 ns 0.88 0.01 2356 B 0.0057 - - 48 B
Utf16StringBuilderAppendFormat_ x:{0}, y:{1} 144.91 ns 0.756 ns 0.707 ns 1.00 0.00 7276 B - - - -
Utf16StringBuilderAppendFormatN x:{0}, y:{1} 102.43 ns 0.460 ns 0.431 ns 0.71 0.00 7393 B - - - -
Utf8StringBuilderAppendFormat_ x:{0}, y:{1} 175.73 ns 0.787 ns 0.736 ns 1.00 0.00 7856 B - - - -
Utf8StringBuilderAppendFormatN x:{0}, y:{1} 147.53 ns 0.988 ns 0.876 ns 0.84 0.01 10549 B - - - -
BenchmarkDotNet=v0.12.1, OS=Windows 10.0.19042
AMD Ryzen 7 3700X, 1 CPU, 16 logical and 8 physical cores
.NET Core SDK=3.1.302
  [Host]     : .NET Core 3.1.6 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.31603), X64 RyuJIT
  DefaultJob : .NET Core 3.1.6 (CoreCLR 4.700.20.26901, CoreFX 4.700.20.31603), X64 RyuJIT

BuiltinTypesBenchmark.cs

Method Mean Error StdDev Ratio Code Size Gen 0 Gen 1 Gen 2 Allocated
Format_ 2,871.2 ns 19.09 ns 17.86 ns 1.00 952 B 0.0648 - - 560 B
FormatN 2,337.8 ns 15.88 ns 14.85 ns 0.81 952 B 0.0648 - - 552 B
CreateUtf16PreparedFormat_ 1,207.0 ns 17.42 ns 16.29 ns 1.00 200 B 0.3700 0.0038 - 3104 B
CreateUtf16PreparedFormatN 536.0 ns 8.16 ns 7.63 ns 0.44 159 B 0.1965 0.0010 - 1648 B
CreateUtf8PreparedFormat_ 1,228.2 ns 19.72 ns 18.45 ns 1.00 203 B 0.3700 0.0038 - 3104 B
CreateUtf8PreparedFormatN 886.5 ns 10.00 ns 8.35 ns 0.72 194 B 0.2699 0.0019 - 2264 B
Utf16PreparedFormat_ 2,349.2 ns 17.56 ns 16.42 ns 1.00 944 B 0.0648 - - 560 B
Utf16PreparedFormatN 2,283.9 ns 18.38 ns 17.19 ns 0.97 944 B 0.0648 - - 560 B
Utf8PreparedFormat_ 1,575.2 ns 10.36 ns 9.69 ns 1.00 961 B 0.0591 - - 504 B
Utf8PreparedFormatN 1,403.4 ns 11.02 ns 10.31 ns 0.89 961 B 0.0591 - - 504 B
Utf16StringBuilderAppendFormat_ 2,814.9 ns 10.21 ns 9.55 ns 1.00 4686 B 0.0076 - - 72 B
Utf16StringBuilderAppendFormatN 2,274.3 ns 18.23 ns 16.16 ns 0.81 5571 B 0.0076 - - 72 B
Utf8StringBuilderAppendFormat_ 2,017.8 ns 16.70 ns 15.62 ns 1.00 5186 B - - - 24 B
Utf8StringBuilderAppendFormatN 1,740.7 ns 10.89 ns 10.19 ns 0.86 6096 B 0.0019 - - 24 B

@udaken
Copy link
Contributor Author

udaken commented Aug 7, 2020

The unit test passes, but I don't know why build-unity is failing.
(I have no knowledge of unity.)

@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2020

This PR is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 7 days.

@github-actions github-actions bot added the stale label Nov 6, 2020
@udaken
Copy link
Contributor Author

udaken commented Nov 9, 2020

Hi @neuecc, I hope you will review this PR.

@github-actions github-actions bot removed the stale label Nov 10, 2020
@neuecc
Copy link
Member

neuecc commented Jan 8, 2021

sorry for delayed response.
Thanks!
This is well written and great achievement.

@neuecc neuecc merged commit ae478c5 into Cysharp:master Jan 8, 2021
@udaken udaken deleted the rewrite-format-parser branch January 18, 2021 13:47
ThangwLee pushed a commit to WolffunGame/ZString that referenced this pull request May 26, 2023
Improved compatibility of Format String
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants