Skip to content

Conversation

@carlossanlop
Copy link
Contributor

The purpose of this PR is to verify the performance of the recent AdvSimd.Arm64 improvements added to Utf16Utility.Validation.cs.

The UTF8Encoding.GetByteCount method reaches the Utf16Utility.Validation.cs lines 82, 150 and 190, when called with the contents of the recently added text files containing long unicode strings.

cc @kunalspathak @pgovind @adamsitnik

Copy link
Contributor

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@carlossanlop
Copy link
Contributor Author

Results:

SSE2 OFF vs SSE2 ON

Off: $Env:COMPlus_EnableSSE2=0
On: $Env:COMPlus_EnableSSE2=1

❯ D:\runtime\.dotnet\dotnet.exe run --base C:\users\carlos\Desktop\sse_off\ --diff C:\users\carlos\Desktop\sse_on\ --threshold 0.001%
summary:
better: 20, geomean: 1.854
total diff: 20

No Slower results for the provided threshold = 0.001% and noise filter = 0.3ns.

| Faster                                                                | base/diff | Base Median (ns) | Diff Median (ns) | Modality|
| --------------------------------------------------------------------- | ---------:| ----------------:| ----------------:| --------:|
| System.Text.Perf_Utf8Encoding.GetByteCount(Input: Greek)              |     10.49 |        154142.40 |         14700.10 |         |
| System.Text.Perf_Utf8Encoding.GetByteCount(Input: Cyrillic)           |      8.97 |        104052.54 |         11601.99 |         |
| System.Text.Perf_Utf8Encoding.GetByteCount(Input: Chinese)            |      5.33 |         72346.37 |         13583.33 |         |
| System.Text.Perf_Utf8Encoding.GetByteCount(Input: EnglishAllAscii)    |      4.04 |         29311.31 |          7260.16 |         |
| System.Text.Perf_Utf8Encoding.GetByteCount(Input: EnglishMostlyAscii) |      3.35 |         98971.88 |         29549.56 |         |

AdvSimd OFF vs AdvSimd ON

Off: export COMPlus_EnableArm64AdvSimd=0
On: export COMPlus_EnableArm64AdvSimd=1

root@calopearm:/home/calope/performance/src/tools/ResultsComparer# dn run -c release --base /home/calope/arm_off/ --diff /home/calope/arm_on/ --threshold 0.001%summary:
better: 13, geomean: 1.772
total diff: 13

No Slower results for the provided threshold = 0.001% and noise filter = 0.3ns.

| Faster                                                                | base/diff | Base Median (ns) | Diff Median (ns) | Modality|
| --------------------------------------------------------------------- | ---------:| ----------------:| ----------------:| -------- |
| System.Text.Perf_Utf8Encoding.GetByteCount(Input: Greek)              |      3.50 |        295751.19 |         84604.67 |         |
| System.Text.Perf_Utf8Encoding.GetByteCount(Input: Cyrillic)           |      2.76 |        225379.32 |         81806.99 | several?|
| System.Text.Perf_Utf8Encoding.GetByteCount(Input: Chinese)            |      2.52 |        209528.09 |         83099.90 |         |
| System.Text.Perf_Utf8Encoding.GetByteCount(Input: EnglishAllAscii)    |      1.56 |         64962.94 |         41614.67 |         |
| System.Text.Perf_Utf8Encoding.GetByteCount(Input: EnglishMostlyAscii) |      1.16 |        228289.04 |        196492.73 |         |

Copy link

@pgovind pgovind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Nice numbers!

Copy link
Member

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you @carlossanlop !

@adamsitnik adamsitnik merged commit 6e457b3 into dotnet:master Aug 24, 2020
@pgovind
Copy link

pgovind commented Aug 24, 2020

I'm just wondering if this will break the mono CI too.

@pgovind
Copy link

pgovind commented Aug 24, 2020

Maybe not. I just saw that @adamsitnik fixed the mono issue with a NoMono category. Nice!

@adamsitnik
Copy link
Member

I'm just wondering if this will break the mono CI too.

This one not because it's not using the forbidden constructor :D

@carlossanlop carlossanlop deleted the encoding branch March 9, 2023 03:03
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.

4 participants