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

正規表現をコンパイル済みみして高速化 #335

Merged
merged 3 commits into from
Aug 15, 2024

Conversation

ueno1969
Copy link
Contributor

issue #322 に関して正規表現の部分をコンパイル済み正規表現にして高速化してみました。

  • OperationItems.cs の修正で、dbを使ったときの速度は高速化しました。
    • 手元の環境では、dbの8000行が2.9sから0.9sになっています。
  • 自作のプログラムのアセンブルをしたところ、まだ時間がかかっていたので、影響の大きそうな AIName.cs も同様に修正したところ高速化しました。

C#はHello Worldしかやったことなく流儀を知らず、AI頼みの修正になっているのでこれで良いかわかりませんが、プルリクエストをお送りします。
ビルドも、よく理解していないのですが、 dotnet build でビルドして動作させ、dotnet test でテストも通っているように思います。

失礼な点があればご容赦ください。

@AILight
Copy link
Owner

AILight commented Jul 29, 2024

ありがとうございます。大変助かります。以前にコンパイル済みの正規表現のテストを行ったところ、それほど速度アップにつながらなかったので一度却下したことがありました。内容の確認を行ってからのマージになりますので少しお時間をいただけると助かります。

@AILight
Copy link
Owner

AILight commented Aug 13, 2024

mainブランチでのベンチマーク

*** AILZ80ASM *** Z-80 Assembler, version 1.0.21.0

BenchmarkDotNet v0.13.12, Windows 11 (10.0.22631.3880/23H2/2023Update/SunValley3)
Intel Core i7-8650U CPU 1.90GHz (Kaby Lake R), 1 CPU, 8 logical and 4 physical cores
.NET SDK 8.0.303
  [Host]     : .NET 8.0.7 (8.0.724.31311), X64 RyuJIT AVX2
  DefaultJob : .NET 8.0.7 (8.0.724.31311), X64 RyuJIT AVX2


| Method     | Mean    | Error    | StdDev   |
|----------- |--------:|---------:|---------:|
| Benchmark1 | 8.386 s | 0.1230 s | 0.0961 s |

@AILight
Copy link
Owner

AILight commented Aug 13, 2024

issue/322ブランチでのベンチマーク

*** AILZ80ASM *** Z-80 Assembler, version 1.0.21.0

BenchmarkDotNet v0.13.12, Windows 11 (10.0.22631.3880/23H2/2023Update/SunValley3)
Intel Core i7-8650U CPU 1.90GHz (Kaby Lake R), 1 CPU, 8 logical and 4 physical cores
.NET SDK 8.0.303
  [Host]     : .NET 8.0.7 (8.0.724.31311), X64 RyuJIT AVX2
  DefaultJob : .NET 8.0.7 (8.0.724.31311), X64 RyuJIT AVX2


| Method     | Mean    | Error    | StdDev   |
|----------- |--------:|---------:|---------:|
| Benchmark1 | 3.800 s | 0.0756 s | 0.0742 s |

@AILight
Copy link
Owner

AILight commented Aug 13, 2024

劇的に早くなっていますね。.NET6もしくは8でコンパイル済みの正規表現の扱いがよくなった可能性があるのかもしれません(私の以前の確認方法がよくなった可能性の方が高いとは思いますが)。素晴らしい対応をしていただいてありがとうございます。UnitTestでの確認を行いましたが、コードをすべて確認していませんので確認後にマージさせていただきます。

@ueno1969
Copy link
Contributor Author

よくわかりませんが、このあたりで .NET8.0でCompiled Regexが(Compiledでないもの?)いろいろ高速化されているようなことが書かれている感じがします。

https://devblogs.microsoft.com/dotnet/performance-improvements-in-net-8/#regex

@AILight
Copy link
Owner

AILight commented Aug 15, 2024

よくわかりませんが、このあたりで .NET8.0でCompiled Regexが(Compiledでないもの?)いろいろ高速化されているようなことが書かれている感じがします。

https://devblogs.microsoft.com/dotnet/performance-improvements-in-net-8/#regex

昔の環境で再テストをする元気がないので(笑)当時評価した結果から想像すると、オプティマイザがCompiled にできるケースの場合はCompiled にしていたか、Compiled のパフォーマンスがそれほどチューニングされていなかったかのどちらかではないかと思っています。自分が評価した時には、前者の挙動っぽいとは思っていました。

このパフォーマンスドキュメントホント面白いですよね。最近さぼって読んでなかったので後で読ませていただきます。

@AILight
Copy link
Owner

AILight commented Aug 15, 2024

ソースコードを確認させていただきました。もともとのソースコードに不具合があった可能性もありますので、それを含めて実際の動作を行いながら評価をさせていただきます。

[issues-332] db を使ったときのアセンブル速度について
@AILight
Copy link
Owner

AILight commented Aug 15, 2024

マージありがとうございました。こちらもマージさせていただきます。ありがとうございました。

@AILight AILight self-requested a review August 15, 2024 16:49
AILZ80ASM/OperationItems/OperationItemData.cs Outdated Show resolved Hide resolved
AILZ80ASM/OperationItems/OperationItemData.cs Outdated Show resolved Hide resolved
AILZ80ASM/OperationItems/OperationItemData.cs Outdated Show resolved Hide resolved
AILZ80ASM/AILight/AIValue.cs Show resolved Hide resolved
AILZ80ASM/AILight/AIValue.cs Show resolved Hide resolved
AILZ80ASM/AILight/AIValue.cs Show resolved Hide resolved
@AILight AILight merged commit e39b8be into AILight:develop Aug 15, 2024
1 check passed
@ueno1969 ueno1969 deleted the issue/322 branch October 3, 2024 01:41
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