Enable escape analysis and use in vn - #103148

Closed
AndyAyersMS wants to merge 7 commits into
dotnet:mainfrom
AndyAyersMS:EnableEscapeAnalysisAndUseInVN
Closed

Enable escape analysis and use in vn#103148
AndyAyersMS wants to merge 7 commits into
dotnet:mainfrom
AndyAyersMS:EnableEscapeAnalysisAndUseInVN

Conversation

@AndyAyersMS

@AndyAyersMSAndyAyersMS commented Jun 7, 2024

Copy link
Copy Markdown
Member

Note this does not enable stack allocation of ref classes—the aim is to let downstream phases treat the memory from non-escaping objects more aggressively, since it's not exposed cross-thread.

@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 7, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

FYI @EgorBo@jakobbotsch@SingleAccretion

This allows a limited amount of propagation for fields of non-escaping heap objects.

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

Also I am tempted to say that non-escaping news have no effect on the global heap state... still thinking about that one.

Finally I'd like to enable dead store for fields of nonescaping heap objects (with the hope that perhaps we eventually can remove all reads and writes, and then somehow realize the allocation itself is dead, and remove that too).

VN based dead store only considers local defs, and it's not structured in a way that generalizes to field defs. I was going to tack the heap based version on here anyways, and only run it if (a) we have nonescaping news; (b) something (perhaps VN) detects that there are heap stores that are not needed.

In a related set of changes, I have support for adding field seqs to boxes, which would light up all of the above for boxes too... I haven't tried running the two together yet.

@jakobbotsch

Copy link
Copy Markdown
Member

Is the main benefit here expected to come from boxes? How much work do you think it would be to enable the more general object stack allocation for boxes and have this fall out from physical promotion instead?

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

This sounds similar to #96942. When I looked at it I didn't immediately see how to represent this in VN, since memory is handled on a field-by-field basis.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

Test failure is the object allocation test, we now stack allocate in one more case (because of non-escaping helper info).

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

This sounds similar to #96942. When I looked at it I didn't immediately see how to represent this in VN, since memory is handled on a field-by-field basis.

We know the class, so we could enumerate the fields and add zero stores to each field, but that seems a bit clunky.

@AndyAyersMS

AndyAyersMS commented Jun 7, 2024

Copy link
Copy Markdown
MemberAuthor

Stats on asp.net are pretty dire -- for optimized methods

57554 allocation sites
87 non-escaping

And just one method with a diff.

@hez2010

hez2010 commented Jun 7, 2024

Copy link
Copy Markdown
Contributor

The current analysis is pretty limited, it even won't account for cases like:

varb=newB(42);vara=newA(b);// the current analysis considers b being escapedConsole.WriteLine(a.Inner.V);recordA(BInner);recordB(intV);

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

The current analysis is pretty limited,

It is, but there is also some healthy skepticism that it's fundamentally never going to be very good. However, I am surprised there are so few non-escaping allocations. I would at least expect to see a more sizeable number of non-escaping boxes in generic code. So we'll need to dig in somehow and see what's up.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

Escape analysis has indeed gotten a bit stale, with some small fixes the number of non-escaping news increased from 87 to 153. If I break it down by ref class / value class, I lose a lot of SPMI contexts (since we seemingly don't ask for the class attribs all that often), but of the contexts that survive, I see

15172 ref class news
9 don't escape
2012 value class news
27 don't escape

So perhaps a bit more encouraging: if we can show 1% of boxes don't escape... including the one from #9118, perhaps it is worth pushing further.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 7, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@AndyAyersMS@jakobbotsch@hez2010
, '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

Enable escape analysis and use in vn - #103148

Closed
AndyAyersMS wants to merge 7 commits into
dotnet:mainfrom
AndyAyersMS:EnableEscapeAnalysisAndUseInVN
Closed

Enable escape analysis and use in vn#103148
AndyAyersMS wants to merge 7 commits into
dotnet:mainfrom
AndyAyersMS:EnableEscapeAnalysisAndUseInVN

Conversation

@AndyAyersMS

@AndyAyersMSAndyAyersMS commented Jun 7, 2024

Copy link
Copy Markdown
Member

Note this does not enable stack allocation of ref classes—the aim is to let downstream phases treat the memory from non-escaping objects more aggressively, since it's not exposed cross-thread.

@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 7, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

FYI @EgorBo@jakobbotsch@SingleAccretion

This allows a limited amount of propagation for fields of non-escaping heap objects.

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

Also I am tempted to say that non-escaping news have no effect on the global heap state... still thinking about that one.

Finally I'd like to enable dead store for fields of nonescaping heap objects (with the hope that perhaps we eventually can remove all reads and writes, and then somehow realize the allocation itself is dead, and remove that too).

VN based dead store only considers local defs, and it's not structured in a way that generalizes to field defs. I was going to tack the heap based version on here anyways, and only run it if (a) we have nonescaping news; (b) something (perhaps VN) detects that there are heap stores that are not needed.

In a related set of changes, I have support for adding field seqs to boxes, which would light up all of the above for boxes too... I haven't tried running the two together yet.

@jakobbotsch

Copy link
Copy Markdown
Member

Is the main benefit here expected to come from boxes? How much work do you think it would be to enable the more general object stack allocation for boxes and have this fall out from physical promotion instead?

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

This sounds similar to #96942. When I looked at it I didn't immediately see how to represent this in VN, since memory is handled on a field-by-field basis.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

Test failure is the object allocation test, we now stack allocate in one more case (because of non-escaping helper info).

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

This sounds similar to #96942. When I looked at it I didn't immediately see how to represent this in VN, since memory is handled on a field-by-field basis.

We know the class, so we could enumerate the fields and add zero stores to each field, but that seems a bit clunky.

@AndyAyersMS

AndyAyersMS commented Jun 7, 2024

Copy link
Copy Markdown
MemberAuthor

Stats on asp.net are pretty dire -- for optimized methods

57554 allocation sites
87 non-escaping

And just one method with a diff.

@hez2010

hez2010 commented Jun 7, 2024

Copy link
Copy Markdown
Contributor

The current analysis is pretty limited, it even won't account for cases like:

varb=newB(42);vara=newA(b);// the current analysis considers b being escapedConsole.WriteLine(a.Inner.V);recordA(BInner);recordB(intV);

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

The current analysis is pretty limited,

It is, but there is also some healthy skepticism that it's fundamentally never going to be very good. However, I am surprised there are so few non-escaping allocations. I would at least expect to see a more sizeable number of non-escaping boxes in generic code. So we'll need to dig in somehow and see what's up.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

Escape analysis has indeed gotten a bit stale, with some small fixes the number of non-escaping news increased from 87 to 153. If I break it down by ref class / value class, I lose a lot of SPMI contexts (since we seemingly don't ask for the class attribs all that often), but of the contexts that survive, I see

15172 ref class news
9 don't escape
2012 value class news
27 don't escape

So perhaps a bit more encouraging: if we can show 1% of boxes don't escape... including the one from #9118, perhaps it is worth pushing further.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 7, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@AndyAyersMS@jakobbotsch@hez2010
, '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

Enable escape analysis and use in vn - #103148

Closed
AndyAyersMS wants to merge 7 commits into
dotnet:mainfrom
AndyAyersMS:EnableEscapeAnalysisAndUseInVN
Closed

Enable escape analysis and use in vn#103148
AndyAyersMS wants to merge 7 commits into
dotnet:mainfrom
AndyAyersMS:EnableEscapeAnalysisAndUseInVN

Conversation

@AndyAyersMS

@AndyAyersMSAndyAyersMS commented Jun 7, 2024

Copy link
Copy Markdown
Member

Note this does not enable stack allocation of ref classes—the aim is to let downstream phases treat the memory from non-escaping objects more aggressively, since it's not exposed cross-thread.

@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 7, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

FYI @EgorBo@jakobbotsch@SingleAccretion

This allows a limited amount of propagation for fields of non-escaping heap objects.

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

Also I am tempted to say that non-escaping news have no effect on the global heap state... still thinking about that one.

Finally I'd like to enable dead store for fields of nonescaping heap objects (with the hope that perhaps we eventually can remove all reads and writes, and then somehow realize the allocation itself is dead, and remove that too).

VN based dead store only considers local defs, and it's not structured in a way that generalizes to field defs. I was going to tack the heap based version on here anyways, and only run it if (a) we have nonescaping news; (b) something (perhaps VN) detects that there are heap stores that are not needed.

In a related set of changes, I have support for adding field seqs to boxes, which would light up all of the above for boxes too... I haven't tried running the two together yet.

@jakobbotsch

Copy link
Copy Markdown
Member

Is the main benefit here expected to come from boxes? How much work do you think it would be to enable the more general object stack allocation for boxes and have this fall out from physical promotion instead?

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

This sounds similar to #96942. When I looked at it I didn't immediately see how to represent this in VN, since memory is handled on a field-by-field basis.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

Test failure is the object allocation test, we now stack allocate in one more case (because of non-escaping helper info).

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

This sounds similar to #96942. When I looked at it I didn't immediately see how to represent this in VN, since memory is handled on a field-by-field basis.

We know the class, so we could enumerate the fields and add zero stores to each field, but that seems a bit clunky.

@AndyAyersMS

AndyAyersMS commented Jun 7, 2024

Copy link
Copy Markdown
MemberAuthor

Stats on asp.net are pretty dire -- for optimized methods

57554 allocation sites
87 non-escaping

And just one method with a diff.

@hez2010

hez2010 commented Jun 7, 2024

Copy link
Copy Markdown
Contributor

The current analysis is pretty limited, it even won't account for cases like:

varb=newB(42);vara=newA(b);// the current analysis considers b being escapedConsole.WriteLine(a.Inner.V);recordA(BInner);recordB(intV);

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

The current analysis is pretty limited,

It is, but there is also some healthy skepticism that it's fundamentally never going to be very good. However, I am surprised there are so few non-escaping allocations. I would at least expect to see a more sizeable number of non-escaping boxes in generic code. So we'll need to dig in somehow and see what's up.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

Escape analysis has indeed gotten a bit stale, with some small fixes the number of non-escaping news increased from 87 to 153. If I break it down by ref class / value class, I lose a lot of SPMI contexts (since we seemingly don't ask for the class attribs all that often), but of the contexts that survive, I see

15172 ref class news
9 don't escape
2012 value class news
27 don't escape

So perhaps a bit more encouraging: if we can show 1% of boxes don't escape... including the one from #9118, perhaps it is worth pushing further.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 7, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@AndyAyersMS@jakobbotsch@hez2010
, '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

Enable escape analysis and use in vn - #103148

Closed
AndyAyersMS wants to merge 7 commits into
dotnet:mainfrom
AndyAyersMS:EnableEscapeAnalysisAndUseInVN
Closed

Enable escape analysis and use in vn#103148
AndyAyersMS wants to merge 7 commits into
dotnet:mainfrom
AndyAyersMS:EnableEscapeAnalysisAndUseInVN

Conversation

@AndyAyersMS

@AndyAyersMSAndyAyersMS commented Jun 7, 2024

Copy link
Copy Markdown
Member

Note this does not enable stack allocation of ref classes—the aim is to let downstream phases treat the memory from non-escaping objects more aggressively, since it's not exposed cross-thread.

@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 7, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

FYI @EgorBo@jakobbotsch@SingleAccretion

This allows a limited amount of propagation for fields of non-escaping heap objects.

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

Also I am tempted to say that non-escaping news have no effect on the global heap state... still thinking about that one.

Finally I'd like to enable dead store for fields of nonescaping heap objects (with the hope that perhaps we eventually can remove all reads and writes, and then somehow realize the allocation itself is dead, and remove that too).

VN based dead store only considers local defs, and it's not structured in a way that generalizes to field defs. I was going to tack the heap based version on here anyways, and only run it if (a) we have nonescaping news; (b) something (perhaps VN) detects that there are heap stores that are not needed.

In a related set of changes, I have support for adding field seqs to boxes, which would light up all of the above for boxes too... I haven't tried running the two together yet.

@jakobbotsch

Copy link
Copy Markdown
Member

Is the main benefit here expected to come from boxes? How much work do you think it would be to enable the more general object stack allocation for boxes and have this fall out from physical promotion instead?

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

This sounds similar to #96942. When I looked at it I didn't immediately see how to represent this in VN, since memory is handled on a field-by-field basis.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

Test failure is the object allocation test, we now stack allocate in one more case (because of non-escaping helper info).

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

This sounds similar to #96942. When I looked at it I didn't immediately see how to represent this in VN, since memory is handled on a field-by-field basis.

We know the class, so we could enumerate the fields and add zero stores to each field, but that seems a bit clunky.

@AndyAyersMS

AndyAyersMS commented Jun 7, 2024

Copy link
Copy Markdown
MemberAuthor

Stats on asp.net are pretty dire -- for optimized methods

57554 allocation sites
87 non-escaping

And just one method with a diff.

@hez2010

hez2010 commented Jun 7, 2024

Copy link
Copy Markdown
Contributor

The current analysis is pretty limited, it even won't account for cases like:

varb=newB(42);vara=newA(b);// the current analysis considers b being escapedConsole.WriteLine(a.Inner.V);recordA(BInner);recordB(intV);

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

The current analysis is pretty limited,

It is, but there is also some healthy skepticism that it's fundamentally never going to be very good. However, I am surprised there are so few non-escaping allocations. I would at least expect to see a more sizeable number of non-escaping boxes in generic code. So we'll need to dig in somehow and see what's up.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

Escape analysis has indeed gotten a bit stale, with some small fixes the number of non-escaping news increased from 87 to 153. If I break it down by ref class / value class, I lose a lot of SPMI contexts (since we seemingly don't ask for the class attribs all that often), but of the contexts that survive, I see

15172 ref class news
9 don't escape
2012 value class news
27 don't escape

So perhaps a bit more encouraging: if we can show 1% of boxes don't escape... including the one from #9118, perhaps it is worth pushing further.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 7, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@AndyAyersMS@jakobbotsch@hez2010
, '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

Enable escape analysis and use in vn - #103148

Closed
AndyAyersMS wants to merge 7 commits into
dotnet:mainfrom
AndyAyersMS:EnableEscapeAnalysisAndUseInVN
Closed

Enable escape analysis and use in vn#103148
AndyAyersMS wants to merge 7 commits into
dotnet:mainfrom
AndyAyersMS:EnableEscapeAnalysisAndUseInVN

Conversation

@AndyAyersMS

@AndyAyersMSAndyAyersMS commented Jun 7, 2024

Copy link
Copy Markdown
Member

Note this does not enable stack allocation of ref classes—the aim is to let downstream phases treat the memory from non-escaping objects more aggressively, since it's not exposed cross-thread.

@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 7, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

FYI @EgorBo@jakobbotsch@SingleAccretion

This allows a limited amount of propagation for fields of non-escaping heap objects.

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

Also I am tempted to say that non-escaping news have no effect on the global heap state... still thinking about that one.

Finally I'd like to enable dead store for fields of nonescaping heap objects (with the hope that perhaps we eventually can remove all reads and writes, and then somehow realize the allocation itself is dead, and remove that too).

VN based dead store only considers local defs, and it's not structured in a way that generalizes to field defs. I was going to tack the heap based version on here anyways, and only run it if (a) we have nonescaping news; (b) something (perhaps VN) detects that there are heap stores that are not needed.

In a related set of changes, I have support for adding field seqs to boxes, which would light up all of the above for boxes too... I haven't tried running the two together yet.

@jakobbotsch

Copy link
Copy Markdown
Member

Is the main benefit here expected to come from boxes? How much work do you think it would be to enable the more general object stack allocation for boxes and have this fall out from physical promotion instead?

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

This sounds similar to #96942. When I looked at it I didn't immediately see how to represent this in VN, since memory is handled on a field-by-field basis.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

Test failure is the object allocation test, we now stack allocate in one more case (because of non-escaping helper info).

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

This sounds similar to #96942. When I looked at it I didn't immediately see how to represent this in VN, since memory is handled on a field-by-field basis.

We know the class, so we could enumerate the fields and add zero stores to each field, but that seems a bit clunky.

@AndyAyersMS

AndyAyersMS commented Jun 7, 2024

Copy link
Copy Markdown
MemberAuthor

Stats on asp.net are pretty dire -- for optimized methods

57554 allocation sites
87 non-escaping

And just one method with a diff.

@hez2010

hez2010 commented Jun 7, 2024

Copy link
Copy Markdown
Contributor

The current analysis is pretty limited, it even won't account for cases like:

varb=newB(42);vara=newA(b);// the current analysis considers b being escapedConsole.WriteLine(a.Inner.V);recordA(BInner);recordB(intV);

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

The current analysis is pretty limited,

It is, but there is also some healthy skepticism that it's fundamentally never going to be very good. However, I am surprised there are so few non-escaping allocations. I would at least expect to see a more sizeable number of non-escaping boxes in generic code. So we'll need to dig in somehow and see what's up.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

Escape analysis has indeed gotten a bit stale, with some small fixes the number of non-escaping news increased from 87 to 153. If I break it down by ref class / value class, I lose a lot of SPMI contexts (since we seemingly don't ask for the class attribs all that often), but of the contexts that survive, I see

15172 ref class news
9 don't escape
2012 value class news
27 don't escape

So perhaps a bit more encouraging: if we can show 1% of boxes don't escape... including the one from #9118, perhaps it is worth pushing further.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 7, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@AndyAyersMS@jakobbotsch@hez2010
, '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

Enable escape analysis and use in vn - #103148

Closed
AndyAyersMS wants to merge 7 commits into
dotnet:mainfrom
AndyAyersMS:EnableEscapeAnalysisAndUseInVN
Closed

Enable escape analysis and use in vn#103148
AndyAyersMS wants to merge 7 commits into
dotnet:mainfrom
AndyAyersMS:EnableEscapeAnalysisAndUseInVN

Conversation

@AndyAyersMS

@AndyAyersMSAndyAyersMS commented Jun 7, 2024

Copy link
Copy Markdown
Member

Note this does not enable stack allocation of ref classes—the aim is to let downstream phases treat the memory from non-escaping objects more aggressively, since it's not exposed cross-thread.

@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 7, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

FYI @EgorBo@jakobbotsch@SingleAccretion

This allows a limited amount of propagation for fields of non-escaping heap objects.

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

Also I am tempted to say that non-escaping news have no effect on the global heap state... still thinking about that one.

Finally I'd like to enable dead store for fields of nonescaping heap objects (with the hope that perhaps we eventually can remove all reads and writes, and then somehow realize the allocation itself is dead, and remove that too).

VN based dead store only considers local defs, and it's not structured in a way that generalizes to field defs. I was going to tack the heap based version on here anyways, and only run it if (a) we have nonescaping news; (b) something (perhaps VN) detects that there are heap stores that are not needed.

In a related set of changes, I have support for adding field seqs to boxes, which would light up all of the above for boxes too... I haven't tried running the two together yet.

@jakobbotsch

Copy link
Copy Markdown
Member

Is the main benefit here expected to come from boxes? How much work do you think it would be to enable the more general object stack allocation for boxes and have this fall out from physical promotion instead?

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

This sounds similar to #96942. When I looked at it I didn't immediately see how to represent this in VN, since memory is handled on a field-by-field basis.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

Test failure is the object allocation test, we now stack allocate in one more case (because of non-escaping helper info).

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

This sounds similar to #96942. When I looked at it I didn't immediately see how to represent this in VN, since memory is handled on a field-by-field basis.

We know the class, so we could enumerate the fields and add zero stores to each field, but that seems a bit clunky.

@AndyAyersMS

AndyAyersMS commented Jun 7, 2024

Copy link
Copy Markdown
MemberAuthor

Stats on asp.net are pretty dire -- for optimized methods

57554 allocation sites
87 non-escaping

And just one method with a diff.

@hez2010

hez2010 commented Jun 7, 2024

Copy link
Copy Markdown
Contributor

The current analysis is pretty limited, it even won't account for cases like:

varb=newB(42);vara=newA(b);// the current analysis considers b being escapedConsole.WriteLine(a.Inner.V);recordA(BInner);recordB(intV);

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

The current analysis is pretty limited,

It is, but there is also some healthy skepticism that it's fundamentally never going to be very good. However, I am surprised there are so few non-escaping allocations. I would at least expect to see a more sizeable number of non-escaping boxes in generic code. So we'll need to dig in somehow and see what's up.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

Escape analysis has indeed gotten a bit stale, with some small fixes the number of non-escaping news increased from 87 to 153. If I break it down by ref class / value class, I lose a lot of SPMI contexts (since we seemingly don't ask for the class attribs all that often), but of the contexts that survive, I see

15172 ref class news
9 don't escape
2012 value class news
27 don't escape

So perhaps a bit more encouraging: if we can show 1% of boxes don't escape... including the one from #9118, perhaps it is worth pushing further.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 7, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@AndyAyersMS@jakobbotsch@hez2010
, '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

Enable escape analysis and use in vn - #103148

Closed
AndyAyersMS wants to merge 7 commits into
dotnet:mainfrom
AndyAyersMS:EnableEscapeAnalysisAndUseInVN
Closed

Enable escape analysis and use in vn#103148
AndyAyersMS wants to merge 7 commits into
dotnet:mainfrom
AndyAyersMS:EnableEscapeAnalysisAndUseInVN

Conversation

@AndyAyersMS

@AndyAyersMSAndyAyersMS commented Jun 7, 2024

Copy link
Copy Markdown
Member

Note this does not enable stack allocation of ref classes—the aim is to let downstream phases treat the memory from non-escaping objects more aggressively, since it's not exposed cross-thread.

@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 7, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

FYI @EgorBo@jakobbotsch@SingleAccretion

This allows a limited amount of propagation for fields of non-escaping heap objects.

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

Also I am tempted to say that non-escaping news have no effect on the global heap state... still thinking about that one.

Finally I'd like to enable dead store for fields of nonescaping heap objects (with the hope that perhaps we eventually can remove all reads and writes, and then somehow realize the allocation itself is dead, and remove that too).

VN based dead store only considers local defs, and it's not structured in a way that generalizes to field defs. I was going to tack the heap based version on here anyways, and only run it if (a) we have nonescaping news; (b) something (perhaps VN) detects that there are heap stores that are not needed.

In a related set of changes, I have support for adding field seqs to boxes, which would light up all of the above for boxes too... I haven't tried running the two together yet.

@jakobbotsch

Copy link
Copy Markdown
Member

Is the main benefit here expected to come from boxes? How much work do you think it would be to enable the more general object stack allocation for boxes and have this fall out from physical promotion instead?

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

This sounds similar to #96942. When I looked at it I didn't immediately see how to represent this in VN, since memory is handled on a field-by-field basis.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

Test failure is the object allocation test, we now stack allocate in one more case (because of non-escaping helper info).

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

This sounds similar to #96942. When I looked at it I didn't immediately see how to represent this in VN, since memory is handled on a field-by-field basis.

We know the class, so we could enumerate the fields and add zero stores to each field, but that seems a bit clunky.

@AndyAyersMS

AndyAyersMS commented Jun 7, 2024

Copy link
Copy Markdown
MemberAuthor

Stats on asp.net are pretty dire -- for optimized methods

57554 allocation sites
87 non-escaping

And just one method with a diff.

@hez2010

hez2010 commented Jun 7, 2024

Copy link
Copy Markdown
Contributor

The current analysis is pretty limited, it even won't account for cases like:

varb=newB(42);vara=newA(b);// the current analysis considers b being escapedConsole.WriteLine(a.Inner.V);recordA(BInner);recordB(intV);

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

The current analysis is pretty limited,

It is, but there is also some healthy skepticism that it's fundamentally never going to be very good. However, I am surprised there are so few non-escaping allocations. I would at least expect to see a more sizeable number of non-escaping boxes in generic code. So we'll need to dig in somehow and see what's up.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

Escape analysis has indeed gotten a bit stale, with some small fixes the number of non-escaping news increased from 87 to 153. If I break it down by ref class / value class, I lose a lot of SPMI contexts (since we seemingly don't ask for the class attribs all that often), but of the contexts that survive, I see

15172 ref class news
9 don't escape
2012 value class news
27 don't escape

So perhaps a bit more encouraging: if we can show 1% of boxes don't escape... including the one from #9118, perhaps it is worth pushing further.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 7, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@AndyAyersMS@jakobbotsch@hez2010
, '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

Enable escape analysis and use in vn - #103148

Closed
AndyAyersMS wants to merge 7 commits into
dotnet:mainfrom
AndyAyersMS:EnableEscapeAnalysisAndUseInVN
Closed

Enable escape analysis and use in vn#103148
AndyAyersMS wants to merge 7 commits into
dotnet:mainfrom
AndyAyersMS:EnableEscapeAnalysisAndUseInVN

Conversation

@AndyAyersMS

@AndyAyersMSAndyAyersMS commented Jun 7, 2024

Copy link
Copy Markdown
Member

Note this does not enable stack allocation of ref classes—the aim is to let downstream phases treat the memory from non-escaping objects more aggressively, since it's not exposed cross-thread.

@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Jun 7, 2024
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

FYI @EgorBo@jakobbotsch@SingleAccretion

This allows a limited amount of propagation for fields of non-escaping heap objects.

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

Also I am tempted to say that non-escaping news have no effect on the global heap state... still thinking about that one.

Finally I'd like to enable dead store for fields of nonescaping heap objects (with the hope that perhaps we eventually can remove all reads and writes, and then somehow realize the allocation itself is dead, and remove that too).

VN based dead store only considers local defs, and it's not structured in a way that generalizes to field defs. I was going to tack the heap based version on here anyways, and only run it if (a) we have nonescaping news; (b) something (perhaps VN) detects that there are heap stores that are not needed.

In a related set of changes, I have support for adding field seqs to boxes, which would light up all of the above for boxes too... I haven't tried running the two together yet.

@jakobbotsch

Copy link
Copy Markdown
Member

Is the main benefit here expected to come from boxes? How much work do you think it would be to enable the more general object stack allocation for boxes and have this fall out from physical promotion instead?

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

This sounds similar to #96942. When I looked at it I didn't immediately see how to represent this in VN, since memory is handled on a field-by-field basis.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

Test failure is the object allocation test, we now stack allocate in one more case (because of non-escaping helper info).

I would like VN to know that the backing storage for non-escaping heap object fields is all zero -- any suggestions on the best way to implement that?

This sounds similar to #96942. When I looked at it I didn't immediately see how to represent this in VN, since memory is handled on a field-by-field basis.

We know the class, so we could enumerate the fields and add zero stores to each field, but that seems a bit clunky.

@AndyAyersMS

AndyAyersMS commented Jun 7, 2024

Copy link
Copy Markdown
MemberAuthor

Stats on asp.net are pretty dire -- for optimized methods

57554 allocation sites
87 non-escaping

And just one method with a diff.

@hez2010

hez2010 commented Jun 7, 2024

Copy link
Copy Markdown
Contributor

The current analysis is pretty limited, it even won't account for cases like:

varb=newB(42);vara=newA(b);// the current analysis considers b being escapedConsole.WriteLine(a.Inner.V);recordA(BInner);recordB(intV);

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

The current analysis is pretty limited,

It is, but there is also some healthy skepticism that it's fundamentally never going to be very good. However, I am surprised there are so few non-escaping allocations. I would at least expect to see a more sizeable number of non-escaping boxes in generic code. So we'll need to dig in somehow and see what's up.

@AndyAyersMS

Copy link
Copy Markdown
MemberAuthor

Escape analysis has indeed gotten a bit stale, with some small fixes the number of non-escaping news increased from 87 to 153. If I break it down by ref class / value class, I lose a lot of SPMI contexts (since we seemingly don't ask for the class attribs all that often), but of the contexts that survive, I see

15172 ref class news
9 don't escape
2012 value class news
27 don't escape

So perhaps a bit more encouraging: if we can show 1% of boxes don't escape... including the one from #9118, perhaps it is worth pushing further.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.

@github-actionsgithub-actionsBot locked and limited conversation to collaborators Aug 7, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@AndyAyersMS@jakobbotsch@hez2010