') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); Do not use `RhGetCodeTarget` in delegate equality by SingleAccretion · Pull Request #88611 · dotnet/runtime · GitHub
Skip to content

Do not use RhGetCodeTarget in delegate equality - #88611

Merged
jkotas merged 1 commit into
dotnet:mainfrom
SingleAccretion:RhGetCodeTarget-DelEq
Jul 12, 2023
Merged

Do not use RhGetCodeTarget in delegate equality#88611
jkotas merged 1 commit into
dotnet:mainfrom
SingleAccretion:RhGetCodeTarget-DelEq

Conversation

@SingleAccretion

@SingleAccretionSingleAccretion commented Jul 10, 2023•
edited
Loading

Copy link
Copy Markdown
Contributor

dotnet/corert@08d78ae

The original motivation for this was handling import stubs:

Function pointer equality comparison was not handling cross-module pointers correctly when optimizations were enabled
(causes target pointers to be wrapped in jump stubs sometimes). The delegate equality comparison was hitting this bug.

We do not have import stubs anymore and unwrapping unboxing stubs serves no purpose here.

Microbenchmarks of delegate equality show ~3x improvement with this change:

Bench_DelegateEquality_Positive_OpenStatic<10000000>() took: 355 ms
Bench_DelegateEquality_Positive_ClosedStatic<10000000>() took: 367 ms
Bench_DelegateEquality_Positive_ClosedInstance<10000000>() took: 371 ms
Bench_DelegateEquality_Positive_OpenStatic<10000000>() took: 121 ms
Bench_DelegateEquality_Positive_ClosedStatic<10000000>() took: 120 ms
Bench_DelegateEquality_Positive_ClosedInstance<10000000>() took: 122 ms

Additionally, there is some desire to upstream changes for a portable RhGetCodeTarget implementation. Not having to deal with it at this relatively low-level layer will make things more robust.

Ref: dotnet/runtimelab#2333.

@SingleAccretion
Do not use RhGetCodeTarget in delegate equality
bd8efd0
dotnet/corert@08d78ae
The original motivation for this was handling import stubs:
```
Function pointer equality comparison was not handling cross-module pointers correctly when optimizations were enabled
(causes target pointers to be wrapped in jump stubs sometimes). The delegate equality comparison was hitting this bug.
```
We do not have import stubs anymore and unwrapping unboxing stubs serves no purpose here.
Microbenchmarks of delegate equality show ~3x improvement with this change:
```
Bench_DelegateEquality_Positive_OpenStatic<10000000>() took: 355 ms
Bench_DelegateEquality_Positive_ClosedStatic<10000000>() took: 367 ms
Bench_DelegateEquality_Positive_ClosedInstance<10000000>() took: 371 ms
Bench_DelegateEquality_Positive_OpenStatic<10000000>() took: 121 ms
Bench_DelegateEquality_Positive_ClosedStatic<10000000>() took: 120 ms
Bench_DelegateEquality_Positive_ClosedInstance<10000000>() took: 122 ms
```
Additionally, there is some desire to upstream changes for a portable RhGetCodeTarget implementation. Not having to
deal with it at this relatively low-level layer will make things more robust.
@ghostghost added area-NativeAOT-coreclr community-contribution Indicates that the PR has been added by a community member labels Jul 10, 2023
@ghost

ghost commented Jul 10, 2023

Copy link
Copy Markdown

Tagging subscribers to this area: @agocke, @MichalStrehovsky, @jkotas
See info in area-owners.md if you want to be subscribed.

Issue Details

dotnet/corert@08d78ae

The original motivation for this was handling import stubs:

Function pointer equality comparison was not handling cross-module pointers correctly when optimizations were enabled
(causes target pointers to be wrapped in jump stubs sometimes). The delegate equality comparison was hitting this bug.

We do not have import stubs anymore and unwrapping unboxing stubs serves no purpose here.

Microbenchmarks of delegate equality show ~3x improvement with this change:

Bench_DelegateEquality_Positive_OpenStatic<10000000>() took: 355 ms
Bench_DelegateEquality_Positive_ClosedStatic<10000000>() took: 367 ms
Bench_DelegateEquality_Positive_ClosedInstance<10000000>() took: 371 ms
Bench_DelegateEquality_Positive_OpenStatic<10000000>() took: 121 ms
Bench_DelegateEquality_Positive_ClosedStatic<10000000>() took: 120 ms
Bench_DelegateEquality_Positive_ClosedInstance<10000000>() took: 122 ms

Additionally, there is some desire to upstream changes for a portable RhGetCodeTarget implementation. Not having to deal with it at this relatively low-level layer will make things more robust.

Author:SingleAccretion
Assignees:-
Labels:

area-NativeAOT-coreclr

Milestone:-

@SingleAccretion
SingleAccretion marked this pull request as ready for review July 10, 2023 19:10
@build-analysisbuild-analysisBot mentioned this pull request Jul 10, 2023
Closed
@jkotas

jkotas commented Jul 11, 2023

Copy link
Copy Markdown
Member

/azp run runtime-extra-platforms

@azure-pipelines

azure-pipelinesBot commented Jul 11, 2023

Copy link
Copy Markdown
Azure Pipelines successfully started running 1 pipeline(s).

This was referenced Jul 11, 2023
Closed
Closed
Closed
Closed
Closed
@SingleAccretion

SingleAccretion commented Jul 11, 2023

Copy link
Copy Markdown
ContributorAuthor

Extra platform NAOT failures are all #88628, except for one System.Private.Xml.Tests failure:

Console log: 'System.Private.Xml.Tests' from job 35ef4761-e9bb-4401-9efe-8cc3f8ab5491 workitem 069930dd-fa36-4f12-a23f-3a80ebbaf30f (windows.amd64.server2022.open.rt) executed on machine a00DJC5 running Windows-10-10.0.20348-SP0
C:\h\w\B10309B3\w\AB6D09C6\e>taskkill.exe /f /im corerun.exe ERROR: The process "corerun.exe" not found.
C:\h\w\B10309B3\w\AB6D09C6\e>call RunTests.cmd --runtime-path C:\h\w\B10309B3\p ----- start Tue 07/11/2023 8:14:20.54 =============== To repro directly: =====================================================
pushd C:\h\w\B10309B3\w\AB6D09C6\e\
System.Private.Xml.Tests.exe -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing -xml testResults.xml popd
===========================================================================================================
C:\h\w\B10309B3\w\AB6D09C6\e>System.Private.Xml.Tests.exe -notrait category=IgnoreForCI -notrait category=OuterLoop -notrait category=failing -xml testResults.xml Running assembly:System.Private.Xml.Tests, Version=8.0.0.0, Culture=neutral, PublicKeyToken=cc7b13ffcd2ddd51
----- end Tue 07/11/2023 8:14:43.33 ----- exit code 3 ----------------------------------------------------------
2023-07-11T08:14:44.090Z	INFO run.py	run(48)	main	Beginning reading of test results.
2023-07-11T08:14:44.091Z	INFO run.py	__init__(42)	read_results	Searching 'C:\h\w\B10309B3\w\AB6D09C6\e' for test results files
2023-07-11T08:14:44.100Z	INFO run.py	__init__(42)	read_results	Searching 'C:\h\w\B10309B3\w\AB6D09C6\uploads' for test results files
2023-07-11T08:14:44.101Z	WARNING	run.py	__init__(55)	read_results	No results file found in any of the following formats: xunit, junit, trx
2023-07-11T08:14:44.101Z	INFO run.py	packing_test_reporter(30)	report_results	Packing 0 test reports to 'C:\h\w\B10309B3\w\AB6D09C6\e\__test_report.json'
2023-07-11T08:14:44.103Z	INFO run.py	packing_test_reporter(33)	report_results	Packed 1551 bytes
ERROR: The process "corerun.exe" not found.
Did not find dumps, skipping dump docs generation.
['System.Private.Xml.Tests' END OF WORK ITEM LOG: Command exited with 3]

@build-analysisbuild-analysisBot mentioned this pull request Jul 11, 2023
Closed
@SingleAccretion

SingleAccretion commented Jul 11, 2023

Copy link
Copy Markdown
ContributorAuthor

except for one System.Private.Xml.Tests failure

Which did not reproduce locally. Not clear what to do about it.

@MichalStrehovsky

MichalStrehovsky commented Jul 12, 2023

Copy link
Copy Markdown
Member

except for one System.Private.Xml.Tests failure

Which did not reproduce locally. Not clear what to do about it.

I re-triggered the leg. I can't explain exit code 3.

jkotas
jkotas approved these changes Jul 12, 2023
View reviewed changes

@jkotasjkotas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you!

@jkotas
jkotas merged commit 6969e7e into dotnet:mainJul 12, 2023
This was referenced Jul 12, 2023
Open
Closed
Closed
@ghostghost locked as resolved and limited conversation to collaborators Aug 13, 2023
@SingleAccretion
SingleAccretion deleted the RhGetCodeTarget-DelEq branch September 28, 2023 18:12
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@jkotasjkotasjkotas approved these changes
@MichalStrehovskyMichalStrehovskyAwaiting requested review from MichalStrehovskyMichalStrehovsky is a code owner

Assignees

No one assigned

Labels

area-NativeAOT-coreclrcommunity-contributionIndicates that the PR has been added by a community member

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@SingleAccretion@jkotas@MichalStrehovsky