Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

CachedTimeSource - Added Thread.MemoryBarrier to avoid code-reordering - #5429

#5429
Merged
snakefoot merged 1 commit into
NLog:devNLog/NLog:devfrom
snakefoot:time-source-cachedsnakefoot/NLog:time-source-cachedCopy head branch name to clipboard
Dec 10, 2023
Merged

CachedTimeSource - Added Thread.MemoryBarrier to avoid code-reordering#5429
snakefoot merged 1 commit into
NLog:devNLog/NLog:devfrom
snakefoot:time-source-cachedsnakefoot/NLog:time-source-cachedCopy head branch name to clipboard

Conversation

@snakefoot

@snakefoot snakefoot commented Nov 28, 2023

Copy link
Copy Markdown
Contributor
  • Verify performance with Benchmark.NET (NET Framework 4.8 + NET6 + force 32bit + force 64bit)

@snakefoot snakefoot added the enhancement Improvement on existing feature label Nov 28, 2023
@snakefoot

snakefoot commented Dec 2, 2023

Copy link
Copy Markdown
Contributor Author

Before without Interlocked.Exchange on 32bit target-platform:

| Method                  | Job        | Platform | Mean       | Error     | StdDev    | Rank | Allocated |
|------------------------ |----------- |--------- |-----------:|----------:|----------:|-----:|----------:|
| FastUtcTimeSource       | Job-JWMLCL | X64      |   4.159 ns | 0.0277 ns | 0.0260 ns |    1 |         - |
| FastLocalTimeSource     | Job-JWMLCL | X64      |   4.688 ns | 0.0306 ns | 0.0286 ns |    2 |         - |
| FastLocalTimeSource     | Job-BNYUHE | X86      |   5.910 ns | 0.0590 ns | 0.0523 ns |    3 |         - |
| FastUtcTimeSource       | Job-BNYUHE | X86      |   5.912 ns | 0.0326 ns | 0.0289 ns |    3 |         - |
| AccurateUtcTimeSource   | Job-JWMLCL | X64      |  67.685 ns | 0.4201 ns | 0.3508 ns |    4 |         - |
| AccurateUtcTimeSource   | Job-BNYUHE | X86      | 134.895 ns | 1.0986 ns | 1.0277 ns |    5 |         - |
| AccurateLocalTimeSource | Job-JWMLCL | X64      | 276.942 ns | 1.6234 ns | 1.2674 ns |    6 |         - |
| AccurateLocalTimeSource | Job-BNYUHE | X86      | 780.794 ns | 4.7053 ns | 4.4013 ns |    7 |         - |

After with Interlocked.Exchange on 32bit target-platform:

| Method                  | Job        | Platform | Mean       | Error     | StdDev    | Rank | Allocated |
|------------------------ |----------- |--------- |-----------:|----------:|----------:|-----:|----------:|
| FastLocalTimeSource     | Job-JWMLCL | X64      |   5.967 ns | 0.0547 ns | 0.0512 ns |    1 |         - |
| FastUtcTimeSource       | Job-JWMLCL | X64      |   7.206 ns | 0.0421 ns | 0.0393 ns |    2 |         - |
| FastLocalTimeSource     | Job-BNYUHE | X86      |  21.429 ns | 0.0661 ns | 0.0618 ns |    3 |         - |
| FastUtcTimeSource       | Job-BNYUHE | X86      |  21.454 ns | 0.0917 ns | 0.0858 ns |    3 |         - |
| AccurateUtcTimeSource   | Job-JWMLCL | X64      |  65.989 ns | 0.3348 ns | 0.2968 ns |    4 |         - |
| AccurateUtcTimeSource   | Job-BNYUHE | X86      | 130.774 ns | 0.4283 ns | 0.3344 ns |    5 |         - |
| AccurateLocalTimeSource | Job-JWMLCL | X64      | 267.692 ns | 1.1005 ns | 1.0294 ns |    6 |         - |
| AccurateLocalTimeSource | Job-BNYUHE | X86      | 759.154 ns | 3.7541 ns | 3.5116 ns |    7 |         - |

So 4 times slower performance-hit when 32bit / x86, but still faster than Accurate-Time-Source.

@snakefoot

snakefoot commented Dec 3, 2023

Copy link
Copy Markdown
Contributor Author

Seems that Intel CPU uses the FPU-registers for handling 64-bit-values, thus even when running in 32-bit-mode then the chance of tearing is minimal. So I guess that is why NLog worked fine without Interlocked on 32-bit platforms

@pull-request-size pull-request-size Bot added size/S and removed size/M labels Dec 7, 2023
@snakefoot snakefoot changed the title TimeSource - DateTime caching using Interlocked on 32 bit platforms CachedTimeSource - Added Thread.MemoryBarrier to avoid code-reordering Dec 7, 2023
@snakefoot
snakefoot force-pushed the time-source-cached branch 2 times, most recently from 8387fd7 to 5da1154 Compare December 7, 2023 18:23
@sonarqubecloud

sonarqubecloud Bot commented Dec 7, 2023

Copy link
Copy Markdown

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@snakefoot

Copy link
Copy Markdown
Contributor Author

Instead performed new benchmark of adding MemoryBarrier to ensure _lastTime is always assigned before _lastTicks.

Method Job Jit Platform Mean Error StdDev
FastUtcTimeSource RyuJitX64 RyuJit X64 3.611 ns 0.0622 ns 0.0582 ns
FastUtcTimeSource LegacyJitX86 LegacyJit X86 5.818 ns 0.0958 ns 0.0896 ns
FastLocalTimeSource RyuJitX64 RyuJit X64 3.590 ns 0.0673 ns 0.0629 ns
FastLocalTimeSource LegacyJitX86 LegacyJit X86 6.047 ns 0.1187 ns 0.1052 ns
PureDateTimeUtcNow RyuJitX64 RyuJit X64 67.222 ns 1.0602 ns 0.9918 ns
PureDateTimeUtcNow LegacyJitX86 LegacyJit X86 136.818 ns 2.1062 ns 1.9702 ns
PureDateTimeLocalNow RyuJitX64 RyuJit X64 271.983 ns 4.2370 ns 3.9633 ns
PureDateTimeLocalNow LegacyJitX86 LegacyJit X86 777.621 ns 13.4024 ns 12.5367 ns
AccurateUtcTimeSource RyuJitX64 RyuJit X64 71.206 ns 1.4384 ns 2.9706 ns
AccurateUtcTimeSource LegacyJitX86 LegacyJit X86 138.488 ns 2.6541 ns 2.4827 ns
AccurateLocalTimeSource RyuJitX64 RyuJit X64 272.284 ns 5.4456 ns 5.0938 ns
AccurateLocalTimeSource LegacyJitX86 LegacyJit X86 779.333 ns 14.0040 ns 13.0993 ns

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

Labels

enhancement Improvement on existing feature size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Morty Proxy This is a proxified and sanitized view of the page, visit original site.