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.

Sign up for GitHub

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

Jump to bottom

Get type members in declaration order #5

Closed
wants to merge 4 commits into from
Closed

Get type members in declaration order #5

wants to merge 4 commits into from

Conversation

buyaa-n
Copy link
Owner

@buyaa-n buyaa-n commented May 10, 2022

No description provided.

@buyaa-n buyaa-n closed this May 12, 2022
@buyaa-n buyaa-n reopened this May 16, 2022
buyaa-n
buyaa-n commented May 16, 2022
View reviewed changes
src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs
@@ -499,6 +499,54 @@ internal void Insert(ref T[] list, string? name, MemberListType listType)
}
}

private void MergeWithGlobalListInOrder(T[] list)
Copy link
Owner Author

Choose a reason for hiding this comment

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

Only called when MemberListType.All, other cases are only requesting one member or a members with same where it comes in order. The existing method MergeWithGlobalList(T[] list) looks more optimal for those cases

@buyaa-n buyaa-n force-pushed the declaration_order branch 2 times, most recently from d4e8a1e to fcf1cab Compare May 17, 2022 23:26
@buyaa-n buyaa-n closed this May 18, 2022
steveharter
steveharter reviewed May 19, 2022
View reviewed changes
src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs
// When all members are requested there should be no element in cache that not found within all members list.
// But somehow in very rare case such element found in cache and not appending that to the all members list
// causing random test failure, must be related to the bug commented on row 597
Array.Resize(ref newCache, length + 1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the +1?

Copy link
Owner Author

@buyaa-n buyaa-n May 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Technically there should be no any extra element, but were random test failure in random projects, I could not really get clear repro or that I could figure out what is going on, so basically followed the existing logic:

runtime/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs

Lines 591 to 602 in 227d564

if (m_cacheComplete)
{
//
// In theory, we should never add more elements to the cache when it is complete.
//
// Unfortunately, we shipped with bugs that cause changes of the complete cache (DevDiv #339308).
// Grow the list by exactly one element in this case to avoid null entries at the end.
//
Debug.Assert(false);
newSize = cachedMembers.Length + 1;

steveharter
steveharter reviewed May 19, 2022
View reviewed changes
src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs

if (!foundInList)
{
newCache[length] = cachedMemberInfo;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Doesn't this only handle 1 "not found" item?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes, if there is one found. In case you did not notified I have created PR in dotnet/runtime with this

buyaa-n pushed a commit that referenced this pull request Jul 6, 2023
@rolfbjarne
[mono][arm64] Handle sysctlbyname failing to find system options. (do…
9b53f4b 
…tnet#87189)

This fixes a startup crash on Big Sur:

> error: * Assertion at /Users/runner/work/1/s/src/mono/mono/utils/mono-hwcap-arm64.c:35, condition `res == 0' not met

Because sysctl can't find some of these options:

    $ sysctl hw.optional.armv8_crc32
    hw.optional.armv8_crc32: 1
    $ sysctl hw.optional.arm.FEAT_RDM
    sysctl: unknown oid 'hw.optional.arm.FEAT_RDM'
    $ sysctl hw.optional.arm.FEAT_DotProd
    sysctl: unknown oid 'hw.optional.arm.FEAT_DotProd'
    $ sysctl hw.optional.arm.FEAT_SHA1
    sysctl: unknown oid 'hw.optional.arm.FEAT_SHA1'
    $ sysctl hw.optional.arm.FEAT_SHA256
    sysctl: unknown oid 'hw.optional.arm.FEAT_SHA256'
    $ sysctl hw.optional.arm.FEAT_AES
    sysctl: unknown oid 'hw.optional.arm.FEAT_AES'

Full stack trace:

* thread #1, queue = 'com.apple.main-thread', stop reason = breakpoint 2.1
  * frame #0: 0x0000010ef37560 libmonosgen-2.0.dylib`monoeg_assertion_message
    frame #1: 0x0000010ef375cc libmonosgen-2.0.dylib`mono_assertion_message + 32
    frame #2: 0x0000010ef40d6c libmonosgen-2.0.dylib`mono_hwcap_arch_init + 544
    frame #3: 0x0000010ef54bd8 libmonosgen-2.0.dylib`mono_hwcap_init + 72
    frame #4: 0x0000010ee14dc0 libmonosgen-2.0.dylib`parse_optimizations + 52
    frame #5: 0x0000010edbed48 libmonosgen-2.0.dylib`mono_init
    frame #6: 0x0000010ee18968 libmonosgen-2.0.dylib`mono_jit_init_version
    frame #7: 0x0000010f48a300 libxamarin-dotnet-debug.dylib`xamarin_bridge_initialize + 216
    frame #8: 0x0000010f4900a4 libxamarin-dotnet-debug.dylib`xamarin_main + 376
@github-actions github-actions bot locked and limited conversation to collaborators Jan 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@steveharter steveharter steveharter left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@buyaa-n @steveharter