Cleanup most usages of Windows file mapping#121896
Cleanup most usages of Windows file mapping#121896huoyaoyuan wants to merge 30 commits intodotnet:mainfrom
Conversation
github-actions
Bot
added
the
needs-area-label
dotnet-policy-service
Bot
added
the
community-contribution
huoyaoyuan
added
area-PAL-coreclr
| #ifdef HOST_WINDOWS | ||
| #define MEMORY_MAPPED_STRESSLOG_BASE_ADDRESS (void*)0x400000000000 | ||
| #else | ||
| #define MEMORY_MAPPED_STRESSLOG_BASE_ADDRESS nullptr |
There was a problem hiding this comment.
Is this removing support for memory mapped stresslog on non-Windows?
There was a problem hiding this comment.
Yes. NativeAOT copy of stresslog removes memory map support on all platforms. I haven't measured the impact.
| if (m_File) | ||
| delete m_File; |
There was a problem hiding this comment.
This can just be delete m_File;. Implementations of delete have handled nullptr for over 30 years.
There was a problem hiding this comment.
All of the existing code here are performing null test for delete. Should we prefer consistency here?
| } | ||
| EX_CATCH | ||
| { | ||
| return nullptr; |
There was a problem hiding this comment.
| return nullptr; | |
| // Swallow all exceptions. |
Co-authored-by: Aaron R Robinson <arobins@microsoft.com>
|
@AaronRobinsonMSFT Can you take another look? |
|
#121896 (comment) is not addressed. Memory mapped stress log has been used for GC development. cc @dotnet/gc @janvorli |
|
What is the motivation for removing the memory mapped stress log?
The memory mapped stress log is a debugging feature that allows accessing a stress log in a running application. I would prefer keeping it working. |
|
The motivation is just to simplify implementation. I can add it back if necessary. |
5 participants
The remaining cases are superpmi (included in #121367) and PEImage, which involves LoadLibrary and we would want more future-proof abstraction.
All the cases included in this PR are transitively used by ilasm/ildasm. For stgio, I'm doing simple ifdef's on OS api, since it's too complicated around different I/O models, and we would get rid of it with new metadata library.