[mono][interp] Remove short branches - #105386

Merged
BrzVlad merged 4 commits into
dotnet:mainfrom
BrzVlad:fix-interp-short-branches
Aug 2, 2024
Merged

[mono][interp] Remove short branches#105386
BrzVlad merged 4 commits into
dotnet:mainfrom
BrzVlad:fix-interp-short-branches

Conversation

@BrzVlad

@BrzVladBrzVlad commented Jul 24, 2024

Copy link
Copy Markdown
Member

They don't really seem to have perf benefits, at the expense of quite a bit of implementation cost with existing issues that are awkward to fix.

@BrzVlad
[mono][interp] Make all branch super instructions operate on long off…
b690835
…sets
Previously they were all short branches without long counterparts. This was buggy and awkward because we tried to compute early conservatively whether a branch is short or long, at superins generation time. The conditions would always hold and we had no fallback if the branch didn't turn out to actually be short. Having a long offset for these branches should have negligible perf impact.
@BrzVlad
[mono][wasm] Fix jiterp for branch superins
95b3ab7
@BrzVlad
[mono][interp] Don't compute native offset estimate during super ins …
189a250
…pass
We generate superins without caring if the branch is long or short since the superinstructions use a long offset now.
@dotnet-policy-service

dotnet-policy-serviceBot commented Jul 24, 2024

Copy link
Copy Markdown
Contributor

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

@BrzVlad
BrzVladforce-pushed the fix-interp-short-branches branch from 057e2bd to 6a54c14CompareJuly 24, 2024 13:42
@lewing
lewing requested a review from kgJuly 24, 2024 14:56
kg
kg approved these changes Jul 24, 2024
View reviewed changes
@BrzVlad
BrzVladforce-pushed the fix-interp-short-branches branch 4 times, most recently from a519562 to ea69694CompareJuly 30, 2024 13:37
@build-analysisbuild-analysisBot mentioned this pull request Jul 30, 2024
Closed
@BrzVlad
[mono][interp] Completely remove short branches
17ed1c3
They don't seem to have any benefit at the expense of a lot of code and added complexity.
@BrzVlad
BrzVladforce-pushed the fix-interp-short-branches branch from ea69694 to 17ed1c3CompareJuly 31, 2024 07:51
This was referenced Jul 31, 2024
Closed
Closed
Closed
Closed
Closed
Closed
Open
@BrzVlad
BrzVlad merged commit ac89819 into dotnet:mainAug 2, 2024
@BrzVlad

BrzVlad commented Aug 2, 2024

Copy link
Copy Markdown
MemberAuthor

@kg It wouldn't hurt to take another look at this, just in case by deleting some of the long short opcodes I'm accidentally breaking generation of some traces. The microbenchmarks should show something anyway if that is the case though.

@kg

kg commented Aug 2, 2024

Copy link
Copy Markdown
Contributor

I'll take a look at the stats on browser-bench, and it should show up in microbenchmarks for sure. Thanks for pushing this across the finish line :)

@lewinglewing mentioned this pull request Aug 6, 2024
Open
@kotlarmiloskotlarmilos mentioned this pull request Aug 7, 2024
Closed
This was referenced Aug 7, 2024
Open
Open
BrzVlad added a commit to BrzVlad/runtime that referenced this pull request Aug 9, 2024
@BrzVlad
Revert "[mono][interp] Remove short branches (dotnet#105386)"
04263e0
This reverts commit ac89819.
BrzVlad added a commit that referenced this pull request Aug 9, 2024
@BrzVlad
[mono][interp] Revert short branch removal (#106178)
07f9843
* Revert "[wasm] Fix long branches in jiterpreter (#106030)"
This reverts commit 9fa8fd5.
* Revert "[mono][interp] Remove short branches (#105386)"
This reverts commit ac89819.
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Sep 2, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@lewinglewingAwaiting requested review from lewinglewing is a code owner
@pavelsavarapavelsavaraAwaiting requested review from pavelsavarapavelsavara is a code owner
@kotlarmiloskotlarmilosAwaiting requested review from kotlarmiloskotlarmilos is a code owner
1 more reviewer
@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@BrzVladBrzVlad

Labels

area-Codegen-Interpreter-mono

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@BrzVlad@kg
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
[mono][interp] Remove short branches - #105386
Merged
BrzVlad merged 4 commits into
dotnet:mainfrom 
BrzVlad:fix-interp-short-branches
Aug 2, 2024
Merged
[mono][interp] Remove short branches#105386
BrzVlad merged 4 commits into
dotnet:mainfrom 
BrzVlad:fix-interp-short-branches
Conversation
@BrzVlad
@BrzVladBrzVlad
commented
Jul 24, 2024

Copy link


Copy Markdown

Member
They don't really seem to have perf benefits, at the expense of quite a bit of implementation cost with existing issues that are awkward to fix.
@BrzVlad
[mono][interp] Make all branch super instructions operate on long off…
b690835
…sets
Previously they were all short branches without long counterparts. This was buggy and awkward because we tried to compute early conservatively whether a branch is short or long, at superins generation time. The conditions would always hold and we had no fallback if the branch didn't turn out to actually be short. Having a long offset for these branches should have negligible perf impact.
@BrzVlad
[mono][wasm] Fix jiterp for branch superins
95b3ab7
@BrzVlad
[mono][interp] Don't compute native offset estimate during super ins …
189a250
…pass
We generate superins without caring if the branch is long or short since the superinstructions use a long offset now.
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 24, 2024

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @BrzVlad, @kotlarmilos

See info in area-owners.md if you want to be subscribed.
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
from
057e2bd to
6a54c14CompareJuly 24, 2024 13:42
@lewing
lewing
requested a review
from kgJuly 24, 2024 14:56
kg
kg
approved these changes
Jul 24, 2024
View reviewed changes
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
4 times, most recently
from
a519562 to
ea69694CompareJuly 30, 2024 13:37
@build-analysisbuild-analysisBot
mentioned this pull request
Jul 30, 2024

Closed

@BrzVlad
[mono][interp] Completely remove short branches
17ed1c3
They don't seem to have any benefit at the expense of a lot of code and added complexity.
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
from
ea69694 to
17ed1c3CompareJuly 31, 2024 07:51

This was referenced Jul 31, 2024

Closed


Closed


Closed


Closed


Closed


Closed


Open

@BrzVlad
BrzVlad
merged commit ac89819
into
dotnet:mainAug 2, 2024
@BrzVlad
BrzVlad
commented
Aug 2, 2024

Copy link


Copy Markdown

MemberAuthor
@kg It wouldn't hurt to take another look at this, just in case by deleting some of the long short opcodes I'm accidentally breaking generation of some traces. The microbenchmarks should show something anyway if that is the case though.
@kg
kg
commented
Aug 2, 2024

Copy link


Copy Markdown

Contributor
I'll take a look at the stats on browser-bench, and it should show up in microbenchmarks for sure. Thanks for pushing this across the finish line :)
@lewinglewing
mentioned this pull request
Aug 6, 2024

Open

@kotlarmiloskotlarmilos
mentioned this pull request
Aug 7, 2024

Closed


This was referenced Aug 7, 2024

Open


Open

BrzVlad added a commit
to BrzVlad/runtime
that referenced
this pull request
Aug 9, 2024
@BrzVlad
Revert "[mono][interp] Remove short branches (dotnet#105386)"
04263e0
This reverts commit ac89819.
BrzVlad added a commit
that referenced
this pull request
Aug 9, 2024
@BrzVlad
[mono][interp] Revert short branch removal (#106178)
07f9843
* Revert "[wasm] Fix long branches in jiterpreter (#106030)"
This reverts commit 9fa8fd5.
* Revert "[mono][interp] Remove short branches (#105386)"
This reverts commit ac89819.
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Sep 2, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@lewinglewingAwaiting requested review from lewinglewing is a code owner
@pavelsavarapavelsavaraAwaiting requested review from pavelsavarapavelsavara is a code owner
@kotlarmiloskotlarmilosAwaiting requested review from kotlarmiloskotlarmilos is a code owner

1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@BrzVladBrzVlad

Labels

area-Codegen-Interpreter-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@BrzVlad@kg

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[mono][interp] Remove short branches - #105386
Merged
BrzVlad merged 4 commits into
dotnet:mainfrom 
BrzVlad:fix-interp-short-branches
Aug 2, 2024
Merged
[mono][interp] Remove short branches#105386
BrzVlad merged 4 commits into
dotnet:mainfrom 
BrzVlad:fix-interp-short-branches
Conversation
@BrzVlad
@BrzVladBrzVlad
commented
Jul 24, 2024

Copy link


Copy Markdown

Member
They don't really seem to have perf benefits, at the expense of quite a bit of implementation cost with existing issues that are awkward to fix.
@BrzVlad
[mono][interp] Make all branch super instructions operate on long off…
b690835
…sets
Previously they were all short branches without long counterparts. This was buggy and awkward because we tried to compute early conservatively whether a branch is short or long, at superins generation time. The conditions would always hold and we had no fallback if the branch didn't turn out to actually be short. Having a long offset for these branches should have negligible perf impact.
@BrzVlad
[mono][wasm] Fix jiterp for branch superins
95b3ab7
@BrzVlad
[mono][interp] Don't compute native offset estimate during super ins …
189a250
…pass
We generate superins without caring if the branch is long or short since the superinstructions use a long offset now.
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 24, 2024

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @BrzVlad, @kotlarmilos

See info in area-owners.md if you want to be subscribed.
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
from
057e2bd to
6a54c14CompareJuly 24, 2024 13:42
@lewing
lewing
requested a review
from kgJuly 24, 2024 14:56
kg
kg
approved these changes
Jul 24, 2024
View reviewed changes
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
4 times, most recently
from
a519562 to
ea69694CompareJuly 30, 2024 13:37
@build-analysisbuild-analysisBot
mentioned this pull request
Jul 30, 2024

Closed

@BrzVlad
[mono][interp] Completely remove short branches
17ed1c3
They don't seem to have any benefit at the expense of a lot of code and added complexity.
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
from
ea69694 to
17ed1c3CompareJuly 31, 2024 07:51

This was referenced Jul 31, 2024

Closed


Closed


Closed


Closed


Closed


Closed


Open

@BrzVlad
BrzVlad
merged commit ac89819
into
dotnet:mainAug 2, 2024
@BrzVlad
BrzVlad
commented
Aug 2, 2024

Copy link


Copy Markdown

MemberAuthor
@kg It wouldn't hurt to take another look at this, just in case by deleting some of the long short opcodes I'm accidentally breaking generation of some traces. The microbenchmarks should show something anyway if that is the case though.
@kg
kg
commented
Aug 2, 2024

Copy link


Copy Markdown

Contributor
I'll take a look at the stats on browser-bench, and it should show up in microbenchmarks for sure. Thanks for pushing this across the finish line :)
@lewinglewing
mentioned this pull request
Aug 6, 2024

Open

@kotlarmiloskotlarmilos
mentioned this pull request
Aug 7, 2024

Closed


This was referenced Aug 7, 2024

Open


Open

BrzVlad added a commit
to BrzVlad/runtime
that referenced
this pull request
Aug 9, 2024
@BrzVlad
Revert "[mono][interp] Remove short branches (dotnet#105386)"
04263e0
This reverts commit ac89819.
BrzVlad added a commit
that referenced
this pull request
Aug 9, 2024
@BrzVlad
[mono][interp] Revert short branch removal (#106178)
07f9843
* Revert "[wasm] Fix long branches in jiterpreter (#106030)"
This reverts commit 9fa8fd5.
* Revert "[mono][interp] Remove short branches (#105386)"
This reverts commit ac89819.
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Sep 2, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@lewinglewingAwaiting requested review from lewinglewing is a code owner
@pavelsavarapavelsavaraAwaiting requested review from pavelsavarapavelsavara is a code owner
@kotlarmiloskotlarmilosAwaiting requested review from kotlarmiloskotlarmilos is a code owner

1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@BrzVladBrzVlad

Labels

area-Codegen-Interpreter-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@BrzVlad@kg

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[mono][interp] Remove short branches - #105386
Merged
BrzVlad merged 4 commits into
dotnet:mainfrom 
BrzVlad:fix-interp-short-branches
Aug 2, 2024
Merged
[mono][interp] Remove short branches#105386
BrzVlad merged 4 commits into
dotnet:mainfrom 
BrzVlad:fix-interp-short-branches
Conversation
@BrzVlad
@BrzVladBrzVlad
commented
Jul 24, 2024

Copy link


Copy Markdown

Member
They don't really seem to have perf benefits, at the expense of quite a bit of implementation cost with existing issues that are awkward to fix.
@BrzVlad
[mono][interp] Make all branch super instructions operate on long off…
b690835
…sets
Previously they were all short branches without long counterparts. This was buggy and awkward because we tried to compute early conservatively whether a branch is short or long, at superins generation time. The conditions would always hold and we had no fallback if the branch didn't turn out to actually be short. Having a long offset for these branches should have negligible perf impact.
@BrzVlad
[mono][wasm] Fix jiterp for branch superins
95b3ab7
@BrzVlad
[mono][interp] Don't compute native offset estimate during super ins …
189a250
…pass
We generate superins without caring if the branch is long or short since the superinstructions use a long offset now.
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 24, 2024

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @BrzVlad, @kotlarmilos

See info in area-owners.md if you want to be subscribed.
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
from
057e2bd to
6a54c14CompareJuly 24, 2024 13:42
@lewing
lewing
requested a review
from kgJuly 24, 2024 14:56
kg
kg
approved these changes
Jul 24, 2024
View reviewed changes
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
4 times, most recently
from
a519562 to
ea69694CompareJuly 30, 2024 13:37
@build-analysisbuild-analysisBot
mentioned this pull request
Jul 30, 2024

Closed

@BrzVlad
[mono][interp] Completely remove short branches
17ed1c3
They don't seem to have any benefit at the expense of a lot of code and added complexity.
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
from
ea69694 to
17ed1c3CompareJuly 31, 2024 07:51

This was referenced Jul 31, 2024

Closed


Closed


Closed


Closed


Closed


Closed


Open

@BrzVlad
BrzVlad
merged commit ac89819
into
dotnet:mainAug 2, 2024
@BrzVlad
BrzVlad
commented
Aug 2, 2024

Copy link


Copy Markdown

MemberAuthor
@kg It wouldn't hurt to take another look at this, just in case by deleting some of the long short opcodes I'm accidentally breaking generation of some traces. The microbenchmarks should show something anyway if that is the case though.
@kg
kg
commented
Aug 2, 2024

Copy link


Copy Markdown

Contributor
I'll take a look at the stats on browser-bench, and it should show up in microbenchmarks for sure. Thanks for pushing this across the finish line :)
@lewinglewing
mentioned this pull request
Aug 6, 2024

Open

@kotlarmiloskotlarmilos
mentioned this pull request
Aug 7, 2024

Closed


This was referenced Aug 7, 2024

Open


Open

BrzVlad added a commit
to BrzVlad/runtime
that referenced
this pull request
Aug 9, 2024
@BrzVlad
Revert "[mono][interp] Remove short branches (dotnet#105386)"
04263e0
This reverts commit ac89819.
BrzVlad added a commit
that referenced
this pull request
Aug 9, 2024
@BrzVlad
[mono][interp] Revert short branch removal (#106178)
07f9843
* Revert "[wasm] Fix long branches in jiterpreter (#106030)"
This reverts commit 9fa8fd5.
* Revert "[mono][interp] Remove short branches (#105386)"
This reverts commit ac89819.
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Sep 2, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@lewinglewingAwaiting requested review from lewinglewing is a code owner
@pavelsavarapavelsavaraAwaiting requested review from pavelsavarapavelsavara is a code owner
@kotlarmiloskotlarmilosAwaiting requested review from kotlarmiloskotlarmilos is a code owner

1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@BrzVladBrzVlad

Labels

area-Codegen-Interpreter-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@BrzVlad@kg

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
[mono][interp] Remove short branches - #105386
Merged
BrzVlad merged 4 commits into
dotnet:mainfrom 
BrzVlad:fix-interp-short-branches
Aug 2, 2024
Merged
[mono][interp] Remove short branches#105386
BrzVlad merged 4 commits into
dotnet:mainfrom 
BrzVlad:fix-interp-short-branches
Conversation
@BrzVlad
@BrzVladBrzVlad
commented
Jul 24, 2024

Copy link


Copy Markdown

Member
They don't really seem to have perf benefits, at the expense of quite a bit of implementation cost with existing issues that are awkward to fix.
@BrzVlad
[mono][interp] Make all branch super instructions operate on long off…
b690835
…sets
Previously they were all short branches without long counterparts. This was buggy and awkward because we tried to compute early conservatively whether a branch is short or long, at superins generation time. The conditions would always hold and we had no fallback if the branch didn't turn out to actually be short. Having a long offset for these branches should have negligible perf impact.
@BrzVlad
[mono][wasm] Fix jiterp for branch superins
95b3ab7
@BrzVlad
[mono][interp] Don't compute native offset estimate during super ins …
189a250
…pass
We generate superins without caring if the branch is long or short since the superinstructions use a long offset now.
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 24, 2024

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @BrzVlad, @kotlarmilos

See info in area-owners.md if you want to be subscribed.
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
from
057e2bd to
6a54c14CompareJuly 24, 2024 13:42
@lewing
lewing
requested a review
from kgJuly 24, 2024 14:56
kg
kg
approved these changes
Jul 24, 2024
View reviewed changes
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
4 times, most recently
from
a519562 to
ea69694CompareJuly 30, 2024 13:37
@build-analysisbuild-analysisBot
mentioned this pull request
Jul 30, 2024

Closed

@BrzVlad
[mono][interp] Completely remove short branches
17ed1c3
They don't seem to have any benefit at the expense of a lot of code and added complexity.
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
from
ea69694 to
17ed1c3CompareJuly 31, 2024 07:51

This was referenced Jul 31, 2024

Closed


Closed


Closed


Closed


Closed


Closed


Open

@BrzVlad
BrzVlad
merged commit ac89819
into
dotnet:mainAug 2, 2024
@BrzVlad
BrzVlad
commented
Aug 2, 2024

Copy link


Copy Markdown

MemberAuthor
@kg It wouldn't hurt to take another look at this, just in case by deleting some of the long short opcodes I'm accidentally breaking generation of some traces. The microbenchmarks should show something anyway if that is the case though.
@kg
kg
commented
Aug 2, 2024

Copy link


Copy Markdown

Contributor
I'll take a look at the stats on browser-bench, and it should show up in microbenchmarks for sure. Thanks for pushing this across the finish line :)
@lewinglewing
mentioned this pull request
Aug 6, 2024

Open

@kotlarmiloskotlarmilos
mentioned this pull request
Aug 7, 2024

Closed


This was referenced Aug 7, 2024

Open


Open

BrzVlad added a commit
to BrzVlad/runtime
that referenced
this pull request
Aug 9, 2024
@BrzVlad
Revert "[mono][interp] Remove short branches (dotnet#105386)"
04263e0
This reverts commit ac89819.
BrzVlad added a commit
that referenced
this pull request
Aug 9, 2024
@BrzVlad
[mono][interp] Revert short branch removal (#106178)
07f9843
* Revert "[wasm] Fix long branches in jiterpreter (#106030)"
This reverts commit 9fa8fd5.
* Revert "[mono][interp] Remove short branches (#105386)"
This reverts commit ac89819.
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Sep 2, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@lewinglewingAwaiting requested review from lewinglewing is a code owner
@pavelsavarapavelsavaraAwaiting requested review from pavelsavarapavelsavara is a code owner
@kotlarmiloskotlarmilosAwaiting requested review from kotlarmiloskotlarmilos is a code owner

1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@BrzVladBrzVlad

Labels

area-Codegen-Interpreter-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@BrzVlad@kg

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[mono][interp] Remove short branches - #105386
Merged
BrzVlad merged 4 commits into
dotnet:mainfrom 
BrzVlad:fix-interp-short-branches
Aug 2, 2024
Merged
[mono][interp] Remove short branches#105386
BrzVlad merged 4 commits into
dotnet:mainfrom 
BrzVlad:fix-interp-short-branches
Conversation
@BrzVlad
@BrzVladBrzVlad
commented
Jul 24, 2024

Copy link


Copy Markdown

Member
They don't really seem to have perf benefits, at the expense of quite a bit of implementation cost with existing issues that are awkward to fix.
@BrzVlad
[mono][interp] Make all branch super instructions operate on long off…
b690835
…sets
Previously they were all short branches without long counterparts. This was buggy and awkward because we tried to compute early conservatively whether a branch is short or long, at superins generation time. The conditions would always hold and we had no fallback if the branch didn't turn out to actually be short. Having a long offset for these branches should have negligible perf impact.
@BrzVlad
[mono][wasm] Fix jiterp for branch superins
95b3ab7
@BrzVlad
[mono][interp] Don't compute native offset estimate during super ins …
189a250
…pass
We generate superins without caring if the branch is long or short since the superinstructions use a long offset now.
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 24, 2024

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @BrzVlad, @kotlarmilos

See info in area-owners.md if you want to be subscribed.
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
from
057e2bd to
6a54c14CompareJuly 24, 2024 13:42
@lewing
lewing
requested a review
from kgJuly 24, 2024 14:56
kg
kg
approved these changes
Jul 24, 2024
View reviewed changes
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
4 times, most recently
from
a519562 to
ea69694CompareJuly 30, 2024 13:37
@build-analysisbuild-analysisBot
mentioned this pull request
Jul 30, 2024

Closed

@BrzVlad
[mono][interp] Completely remove short branches
17ed1c3
They don't seem to have any benefit at the expense of a lot of code and added complexity.
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
from
ea69694 to
17ed1c3CompareJuly 31, 2024 07:51

This was referenced Jul 31, 2024

Closed


Closed


Closed


Closed


Closed


Closed


Open

@BrzVlad
BrzVlad
merged commit ac89819
into
dotnet:mainAug 2, 2024
@BrzVlad
BrzVlad
commented
Aug 2, 2024

Copy link


Copy Markdown

MemberAuthor
@kg It wouldn't hurt to take another look at this, just in case by deleting some of the long short opcodes I'm accidentally breaking generation of some traces. The microbenchmarks should show something anyway if that is the case though.
@kg
kg
commented
Aug 2, 2024

Copy link


Copy Markdown

Contributor
I'll take a look at the stats on browser-bench, and it should show up in microbenchmarks for sure. Thanks for pushing this across the finish line :)
@lewinglewing
mentioned this pull request
Aug 6, 2024

Open

@kotlarmiloskotlarmilos
mentioned this pull request
Aug 7, 2024

Closed


This was referenced Aug 7, 2024

Open


Open

BrzVlad added a commit
to BrzVlad/runtime
that referenced
this pull request
Aug 9, 2024
@BrzVlad
Revert "[mono][interp] Remove short branches (dotnet#105386)"
04263e0
This reverts commit ac89819.
BrzVlad added a commit
that referenced
this pull request
Aug 9, 2024
@BrzVlad
[mono][interp] Revert short branch removal (#106178)
07f9843
* Revert "[wasm] Fix long branches in jiterpreter (#106030)"
This reverts commit 9fa8fd5.
* Revert "[mono][interp] Remove short branches (#105386)"
This reverts commit ac89819.
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Sep 2, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@lewinglewingAwaiting requested review from lewinglewing is a code owner
@pavelsavarapavelsavaraAwaiting requested review from pavelsavarapavelsavara is a code owner
@kotlarmiloskotlarmilosAwaiting requested review from kotlarmiloskotlarmilos is a code owner

1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@BrzVladBrzVlad

Labels

area-Codegen-Interpreter-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@BrzVlad@kg

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
[mono][interp] Remove short branches - #105386
Merged
BrzVlad merged 4 commits into
dotnet:mainfrom 
BrzVlad:fix-interp-short-branches
Aug 2, 2024
Merged
[mono][interp] Remove short branches#105386
BrzVlad merged 4 commits into
dotnet:mainfrom 
BrzVlad:fix-interp-short-branches
Conversation
@BrzVlad
@BrzVladBrzVlad
commented
Jul 24, 2024

Copy link


Copy Markdown

Member
They don't really seem to have perf benefits, at the expense of quite a bit of implementation cost with existing issues that are awkward to fix.
@BrzVlad
[mono][interp] Make all branch super instructions operate on long off…
b690835
…sets
Previously they were all short branches without long counterparts. This was buggy and awkward because we tried to compute early conservatively whether a branch is short or long, at superins generation time. The conditions would always hold and we had no fallback if the branch didn't turn out to actually be short. Having a long offset for these branches should have negligible perf impact.
@BrzVlad
[mono][wasm] Fix jiterp for branch superins
95b3ab7
@BrzVlad
[mono][interp] Don't compute native offset estimate during super ins …
189a250
…pass
We generate superins without caring if the branch is long or short since the superinstructions use a long offset now.
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 24, 2024

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @BrzVlad, @kotlarmilos

See info in area-owners.md if you want to be subscribed.
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
from
057e2bd to
6a54c14CompareJuly 24, 2024 13:42
@lewing
lewing
requested a review
from kgJuly 24, 2024 14:56
kg
kg
approved these changes
Jul 24, 2024
View reviewed changes
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
4 times, most recently
from
a519562 to
ea69694CompareJuly 30, 2024 13:37
@build-analysisbuild-analysisBot
mentioned this pull request
Jul 30, 2024

Closed

@BrzVlad
[mono][interp] Completely remove short branches
17ed1c3
They don't seem to have any benefit at the expense of a lot of code and added complexity.
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
from
ea69694 to
17ed1c3CompareJuly 31, 2024 07:51

This was referenced Jul 31, 2024

Closed


Closed


Closed


Closed


Closed


Closed


Open

@BrzVlad
BrzVlad
merged commit ac89819
into
dotnet:mainAug 2, 2024
@BrzVlad
BrzVlad
commented
Aug 2, 2024

Copy link


Copy Markdown

MemberAuthor
@kg It wouldn't hurt to take another look at this, just in case by deleting some of the long short opcodes I'm accidentally breaking generation of some traces. The microbenchmarks should show something anyway if that is the case though.
@kg
kg
commented
Aug 2, 2024

Copy link


Copy Markdown

Contributor
I'll take a look at the stats on browser-bench, and it should show up in microbenchmarks for sure. Thanks for pushing this across the finish line :)
@lewinglewing
mentioned this pull request
Aug 6, 2024

Open

@kotlarmiloskotlarmilos
mentioned this pull request
Aug 7, 2024

Closed


This was referenced Aug 7, 2024

Open


Open

BrzVlad added a commit
to BrzVlad/runtime
that referenced
this pull request
Aug 9, 2024
@BrzVlad
Revert "[mono][interp] Remove short branches (dotnet#105386)"
04263e0
This reverts commit ac89819.
BrzVlad added a commit
that referenced
this pull request
Aug 9, 2024
@BrzVlad
[mono][interp] Revert short branch removal (#106178)
07f9843
* Revert "[wasm] Fix long branches in jiterpreter (#106030)"
This reverts commit 9fa8fd5.
* Revert "[mono][interp] Remove short branches (#105386)"
This reverts commit ac89819.
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Sep 2, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@lewinglewingAwaiting requested review from lewinglewing is a code owner
@pavelsavarapavelsavaraAwaiting requested review from pavelsavarapavelsavara is a code owner
@kotlarmiloskotlarmilosAwaiting requested review from kotlarmiloskotlarmilos is a code owner

1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@BrzVladBrzVlad

Labels

area-Codegen-Interpreter-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@BrzVlad@kg

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
[mono][interp] Remove short branches - #105386
Merged
BrzVlad merged 4 commits into
dotnet:mainfrom 
BrzVlad:fix-interp-short-branches
Aug 2, 2024
Merged
[mono][interp] Remove short branches#105386
BrzVlad merged 4 commits into
dotnet:mainfrom 
BrzVlad:fix-interp-short-branches
Conversation
@BrzVlad
@BrzVladBrzVlad
commented
Jul 24, 2024

Copy link


Copy Markdown

Member
They don't really seem to have perf benefits, at the expense of quite a bit of implementation cost with existing issues that are awkward to fix.
@BrzVlad
[mono][interp] Make all branch super instructions operate on long off…
b690835
…sets
Previously they were all short branches without long counterparts. This was buggy and awkward because we tried to compute early conservatively whether a branch is short or long, at superins generation time. The conditions would always hold and we had no fallback if the branch didn't turn out to actually be short. Having a long offset for these branches should have negligible perf impact.
@BrzVlad
[mono][wasm] Fix jiterp for branch superins
95b3ab7
@BrzVlad
[mono][interp] Don't compute native offset estimate during super ins …
189a250
…pass
We generate superins without caring if the branch is long or short since the superinstructions use a long offset now.
@dotnet-policy-service
dotnet-policy-serviceBot
commented
Jul 24, 2024

Copy link


Copy Markdown

Contributor
Tagging subscribers to this area: @BrzVlad, @kotlarmilos

See info in area-owners.md if you want to be subscribed.
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
from
057e2bd to
6a54c14CompareJuly 24, 2024 13:42
@lewing
lewing
requested a review
from kgJuly 24, 2024 14:56
kg
kg
approved these changes
Jul 24, 2024
View reviewed changes
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
4 times, most recently
from
a519562 to
ea69694CompareJuly 30, 2024 13:37
@build-analysisbuild-analysisBot
mentioned this pull request
Jul 30, 2024

Closed

@BrzVlad
[mono][interp] Completely remove short branches
17ed1c3
They don't seem to have any benefit at the expense of a lot of code and added complexity.
@BrzVlad
BrzVladforce-pushed
the
fix-interp-short-branches
branch
from
ea69694 to
17ed1c3CompareJuly 31, 2024 07:51

This was referenced Jul 31, 2024

Closed


Closed


Closed


Closed


Closed


Closed


Open

@BrzVlad
BrzVlad
merged commit ac89819
into
dotnet:mainAug 2, 2024
@BrzVlad
BrzVlad
commented
Aug 2, 2024

Copy link


Copy Markdown

MemberAuthor
@kg It wouldn't hurt to take another look at this, just in case by deleting some of the long short opcodes I'm accidentally breaking generation of some traces. The microbenchmarks should show something anyway if that is the case though.
@kg
kg
commented
Aug 2, 2024

Copy link


Copy Markdown

Contributor
I'll take a look at the stats on browser-bench, and it should show up in microbenchmarks for sure. Thanks for pushing this across the finish line :)
@lewinglewing
mentioned this pull request
Aug 6, 2024

Open

@kotlarmiloskotlarmilos
mentioned this pull request
Aug 7, 2024

Closed


This was referenced Aug 7, 2024

Open


Open

BrzVlad added a commit
to BrzVlad/runtime
that referenced
this pull request
Aug 9, 2024
@BrzVlad
Revert "[mono][interp] Remove short branches (dotnet#105386)"
04263e0
This reverts commit ac89819.
BrzVlad added a commit
that referenced
this pull request
Aug 9, 2024
@BrzVlad
[mono][interp] Revert short branch removal (#106178)
07f9843
* Revert "[wasm] Fix long branches in jiterpreter (#106030)"
This reverts commit 9fa8fd5.
* Revert "[mono][interp] Remove short branches (#105386)"
This reverts commit ac89819.
@github-actionsgithub-actionsBot
locked and limited conversation to collaborators
Sep 2, 2024
Sign up for freeto subscribe to this conversation on GitHub.
Already have an account?
Sign in.


Reviewers

@lewinglewingAwaiting requested review from lewinglewing is a code owner
@pavelsavarapavelsavaraAwaiting requested review from pavelsavarapavelsavara is a code owner
@kotlarmiloskotlarmilosAwaiting requested review from kotlarmiloskotlarmilos is a code owner

1 more reviewer

@kgkgkg approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@BrzVladBrzVlad

Labels

area-Codegen-Interpreter-mono

Projects


None yet


Milestone


No milestone


Development

Successfully merging this pull request may close these issues.

2 participants

@BrzVlad@kg