No-Merge [Arm64/Unix] Use sys membarrier #12259
Conversation
|
This patch is close to what is proposed in #1563. Initial testing on Arm64/Unix says this is very bad for performance. Total runtime for 11K tests increased 50%. Kernel spent a lot of time optimizing |
| @@ -167,6 +167,22 @@ check_cxx_symbol_exists(_SC_PHYS_PAGES unistd.h HAVE__SC_PHYS_PAGES) | |||
| check_cxx_symbol_exists(_SC_AVPHYS_PAGES unistd.h HAVE__SC_AVPHYS_PAGES) | |||
|
|
|||
| check_cxx_source_runs(" | |||
| #include <unistd.h> | |||
There was a problem hiding this comment.
This should be a runtime check. We build our portable dotnet core stuff on RHEL / CentOS 7 where this syscall is not present and we still want it to work on machines that has kernel support for this syscall.
| @@ -3035,6 +3039,11 @@ Return | |||
| BOOL | |||
| InitializeFlushProcessWriteBuffers() | |||
| { | |||
| #if HAVE_SYS_MEMBARRIER && defined(_ARM64_) | |||
There was a problem hiding this comment.
As I have mentioned before, we should attempt to call the syscall and if it returns ENOSYS, fall back to the original way. We should set a global flag here so that the actual FlushProcessWriteBuffers knows whether to call the syscall or not. And we would also want to enable it for all architectures, not just ARM64.
|
@sdmaclea oh, I've read your comment after reviewing it. This is unfortunate :-(. |
|
@janvorli It may not be as bad on other architectures. I have not found the arm64 implementation to look at what is done. The <linux/membarrier.h> header should not be used if you want it cross platform. Also will be per platform defines of __NR_membarrier since it may not be present in headers. |
No description provided.