Disable RecyclerWatsonTelemetry and Use RDTSC for GetTickCount #1780
Conversation
obastemur
force-pushed
the
perf
branch
2 times, most recently
from
c72a47b to
c405019
Compare
pal/src/misc/time.cpp
Outdated
| inline size_t rdtsc() | ||
| { | ||
| uint32_t H, L; | ||
| __asm volatile ("rdtsc":"=a"(L), "=d"(H)); |
There was a problem hiding this comment.
Is this for cross plat implementation? I remembered the prefix has 2 more underscores as suffix. Also is it fine to specify 2 variables on x86?
There was a problem hiding this comment.
Yes, it is for xplat and, it is perfectly fine.
obastemur
force-pushed
the
perf
branch
3 times, most recently
from
b1a818c to
3ca2038
Compare
|
@dotnet-bot test Ubuntu ubuntu_linux_debug_static |
|
@dotnet-bot test Ubuntu ubuntu_linux_release_static |
|
@dotnet-bot test Ubuntu ubuntu_linux_debug_static |
1 similar comment
|
@dotnet-bot test Ubuntu ubuntu_linux_debug_static |
lib/Common/Memory/Recycler.cpp
Outdated
| #ifdef HEAP_ENUMERATION_VALIDATION | ||
| ,pfPostHeapEnumScanCallback(nullptr) | ||
| #endif | ||
| #ifndef TARGET_CHAKRACORE |
There was a problem hiding this comment.
Please use existing #ifdef NTBUILD instead of introducing a new one.
There was a problem hiding this comment.
I will but NTBUILD does not sound like this is for ChakraFull build. It reminds me WinNT instead..
There was a problem hiding this comment.
I agree... (and I actually think it is WinNT).
pal/src/misc/time.cpp
Outdated
| #ifdef _X86_ | ||
| return L; | ||
| #else | ||
| return ((ULONGLONG)H << 32) | L; |
There was a problem hiding this comment.
(ULONGLONG)H [](start = 12, length = 12)
nit: H is already ULONGLONG type, why cast
There was a problem hiding this comment.
That was supposed to be uint32 instead but PAL internal has this ULONGLONG stuff. Will update this one
pal/src/misc/time.cpp
Outdated
| return (end - start) / usec; | ||
| } | ||
|
|
||
| static ULONGLONG cpu_speed = CPUFreq() * 1e3; // 1000 + 1e6 => ns to ms |
There was a problem hiding this comment.
1000 + 1e6 [](start = 49, length = 10)
nit: Is comment 1000 + ... meant to be ... * 1000?
There was a problem hiding this comment.
No 1e3 means 1000. Comment is explaining the nanosec -> millisec conversion.
There was a problem hiding this comment.
I meant the char "+" in comment appeared to be an arithmetic operator and confused me.
pal/src/misc/time.cpp
Outdated
| { | ||
| return rdtsc() / cpu_speed; | ||
| } | ||
| GetTickCount64FallbackCB getTickCount64FallbackCB = cpu_speed ? FastTickCount : GetTickCount64Fallback; |
There was a problem hiding this comment.
GetTickCount64FallbackCB getTickCount64FallbackCB [](start = 0, length = 49)
nit: "static" GetTickCount64FallbackCB getTickCount64FallbackCB?
Brings ~4% perf improvement [ http-load test, measured on xplat ] - Disables RecyclerWatsonTelemetry for ChakraCore [ reduces the number of calls to system clock api <=~1.5% ] - Use RDTSC for GetTickCount. [ this affects only xplat <=~3% ]
|
@dotnet-bot test Ubuntu ubuntu_linux_debug_static |
|
@dotnet-bot test Ubuntu ubuntu_linux_release_static |
|
@jianchun @ThomsonTan Thanks for the reviews |
…C for GetTickCount Merge pull request #1780 from obastemur:perf Brings ~4% perf improvement [ http-load test, measured on xplat ] - Disables RecyclerWatsonTelemetry for ChakraCore [ reduces the number of calls to system clock api <=~1.5% ] - Use RDTSC for GetTickCount. [ this affects only xplat <=~3% ]
5 participants
Brings ~4% perf improvement [ http-load test, measured on xplat ]
[ reduces the number of calls to system clock api <=~1.5% ]
[ this affects only xplat <=~3% ]