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

[mono] Cleanup todo items from defines-todo.cmake #66408

Merged
merged 28 commits into from
Mar 11, 2022

Conversation

akoeplinger
Copy link
Member

@akoeplinger akoeplinger commented Mar 9, 2022

There are roughly three categories (all in separate commits so it's easier to review):

  1. Already implemented or completely unused defines
  2. Defines we can remove because they're no longer useful:
  • MAJOR_IN_MKDEV and MAJOR_IN_SYSMACROS
    Not used in the runtime anymore.

  • HAVE_EPOLL and HAVE_KQUEUE
    Not used in the runtime anymore.

  • ANDROID_UNIFIED_HEADERS and HAVE_ANDROID_NDK_VERSION_H
    We're requiring a recent Android NDK now that only supports unified headers.

  • MONO_DL_NEED_USCORE
    It doesn't seem needed anymore.
    dlsym() on macOS automatically prepends an underscore and from what I can find it was mostly for old *BSDs.

  • GLIBC_BEFORE_2_3_4_SCHED_SETAFFINITY
    We don't support such an old glibc anymore.

  • HAVE_SIOCGIFCONF
    We don't use this and related networking code in the runtime anymore.

  • DISABLE_HW_TRAPS
    We have MONO_ARCH_DISABLE_HW_TRAPS instead.

  • MONO_BIG_ARRAYS
    No longer needed and probably broken due to missing BCL changes.

  • ENABLE_MONODROID and replace ENABLE_MONOTOUCH with TARGET_APPLE_MOBILE
    ENABLE_MONODROID was unused and MonoTouch was an earlier name for Xamarin.iOS, replace it with something more descriptive

- HAVE_WORKING_SIGALTSTACK
Not enabled in dotnet/runtime since the beginning.
It was also making crashes harder to investigate because we wouldn't get "normal" stack traces that GDB could understand.
we decided to keep it

  • MONO_JEMALLOC
    We haven't used it and don't plan to.

  • MONO_XEN_OPT
    Xen is not a primary scenario for Mono anymore so we probably don't need this optimization.

  1. Defines which are still useful and are implemented by this PR
  • Fix MONO_KEYWORD_THREAD build and implement HAVE_TLS_MODEL_ATTR

  • Boehm/Null GC support

  • MONO_SMALL_CONFIG

  • USE_MALLOC_FOR_MEMPOOLS

  • ENABLE_DTRACE

  • ENABLE_EXPERIMENT_TIERED and remove dynamic experiments

  • ENABLE_OVERRIDABLE_ALLOCATORS

  • MONO_ARCH_ILP32

  • RISCV_FPABI

  • __mono_ppc64__

  • HAVE_ARMV5/6. There is an open question about what we'll do with HAVE_ARMV7, added a TODO comment.

Not used in the runtime anymore.
Not used in the runtime anymore.
We're requiring a recent Android NDK now that only supports unified headers.
It doesn't seem needed anymore.
dlsym() on macOS automatically prepends an underscore and from what I can find it was mostly for old *BSDs.
We don't support such an old glibc anymore.
We don't use this and related networking code in the runtime anymore.
We have MONO_ARCH_DISABLE_HW_TRAPS instead.
No longer needed and probably broken due to missing BCL changes.
…E_MOBILE

ENABLE_MONODROID was unused and MonoTouch was an earlier name for Xamarin.iOS, replace it with something more descriptive
Not enabled in dotnet/runtime since the beginning.
It was also making crashes harder to investigate because we wouldn't get "normal" stack traces that GDB could understand.
We haven't used it and don't plan to.
Xen is not a primary scenario for Mono anymore so we probably don't need this optimization.
Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks ok.

We discussed offline:

  1. altstack support shoudl stay. We should run a rolling build with it enabled to make sure it doesn't bitrot. /cc @steveisok
  2. null GC is probably not useful. The motivation for it was for porting, but it's often easier to start with sgen and comment out non-functional parts.

This reverts commit 2ce87ce.
We decided to keep it.
We decided we don't need it anymore.
@akoeplinger
Copy link
Member Author

I added altstack back and deleted null GC.

@akoeplinger akoeplinger changed the title Cleanup todo items from defines-todo.cmake [mono] Cleanup todo items from defines-todo.cmake Mar 11, 2022
@akoeplinger akoeplinger merged commit 9de7aaf into dotnet:main Mar 11, 2022
@akoeplinger akoeplinger deleted the cleanups branch March 11, 2022 19:44
@ghost ghost locked as resolved and limited conversation to collaborators Apr 11, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants