[release/7.0] Fixing a bug that causes us to mistakenly demote a gen2 region to gen0 - #83341

Merged
mangod9 merged 1 commit into
release/7.0-stagingfrom
backport/pr-82413-to-release/7.0
Apr 4, 2023
Merged

[release/7.0] Fixing a bug that causes us to mistakenly demote a gen2 region to gen0#83341
mangod9 merged 1 commit into
release/7.0-stagingfrom
backport/pr-82413-to-release/7.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Mar 13, 2023

Copy link
Copy Markdown
Contributor

Backport of #82413 to release/7.0

/cc @mangod9@Maoni0

Customer Impact

An infinite loop in GC. Multiple customers have reported this issue and it happens when a gen0 region that's almost fully occupied (< 24 bytes free) with one giant plug (ie, no free objects at all in the region). This causes allocate_in_condemned_generations to go into an infinite loop because in ephemeral generations we expect short plugs, ie, we should be able to allocate a min free object in front of each plug. And normally we can because when we allocate objects in gen0 we make sure to break up the allocation contexts with min free objects and when we compact into gen1 we form short plugs.

Reported by multiple customers in #80073. While validating we also found that the repro causes another failure which is fixed by #83342

Testing

local functional testing; GC stress testing; also validated by a customer who hit the issue.

Risk

low risk. this would cause a region that happened to be in gen0 by accident to now actually go through our demotion logic which we already do for all other regions anyway.

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

We are seeing a gen0 region that's almost fully occupied (< 24 bytes free) with one giant plug (ie, no free objects at all in the region).
This causes allocate_in_condemned_generations to go into an infinite loop because in ephemeral generations we expect short plugs,
ie, we should be able to allocate a min free object in front of each plug. And normally we can because when we allocate objects in gen0
we make sure to break up the allocation contexts with min free objects and when we compact into gen1 we form short plugs.
We are in this situation when all of the following conditions are true -
+ we did a gen2 compacting GC that generates a pinned plug in a gen2 region almost as big as the whole region. my guess for the reason why there's this giant pinned plug is because that gen2 region was already really compact so when we called allocate_in_condemned_generations on the non pinned plugs that are next to some pinned plugs in it we discovered we can't move the non pinned plugs anyway so we artificially pinned them and formed a giant pinned plug. and during this GC those objects were no longer pinned so we have one giant non pinned plug.
+ this gen2 region needs to be the last region with pinned plugs;
+ this gen2 region hasn't been consumed by allocate_in_condemned_generations yet so it was processed by process_remaining_regions;
Then in process_remaining_regions we'll set the plan_gen_num for that gen2 region to 0 because we are doing
set_region_plan_gen_num_sip (current_region, current_plan_gen_num);
instead of going through the demotion logic to decide whether we should demote this region or not.
@ghostghost added the area-GC-coreclr label Mar 13, 2023
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #82413 to release/7.0

/cc @mangod9@Maoni0

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author:github-actions[bot]
Assignees:-
Labels:

area-GC-coreclr

Milestone:-

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

approved. we will take for consideration in 7.0.x

@jeffschwMSFTjeffschwMSFT added the Servicing-approved Approved for servicing release label Mar 16, 2023
@jeffschwMSFTjeffschwMSFT added this to the 7.0.6 milestone Mar 16, 2023
@carlossanlop

Copy link
Copy Markdown
Contributor

I'm retargeting this PR to the new release/7.0-staging branch, which is the one that we will use from now on for servicing fixes.

Repo maintainers will now be allowed to merge their own servicing PR as long as it meets the requirements:

  • It is approved by Tactics (signaled by adding the Servicing-approved label).
  • It's signed-off by an area owner.
  • The CI is green, or the failures are investigated as unrelated.
  • And if the PR touches an OOB package, the necessary OOB authoring changes are added.

The new process is described here: runtime/docs/project/library-servicing.md.

The infra team will be actively monitoring servicing PRs to ensure all requirements are met and to help with any issues.

Let me know if you have any questions.

@carlossanlop
carlossanlop changed the base branch from release/7.0 to release/7.0-stagingMarch 28, 2023 20:57
@carlossanlopcarlossanlop added Servicing-approved Approved for servicing release and removed Servicing-approved Approved for servicing release labels Mar 30, 2023
@carlossanlop

Copy link
Copy Markdown
Contributor

Reminder: April 10th is the last day to merge backport PRs to ensure they get included in the May Release. PR owners are now in charge of merging their own PRs.

@mangod9
mangod9 merged commit f8777ff into release/7.0-stagingApr 4, 2023
@carlossanlop
carlossanlop deleted the backport/pr-82413-to-release/7.0 branch April 4, 2023 21:20
@ghostghost locked as resolved and limited conversation to collaborators May 5, 2023
@leecowleecow modified the milestones: 7.0.6, 7.0.7Jun 1, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-GC-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@carlossanlop@jeffschwMSFT@mangod9@leecow
, '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

[release/7.0] Fixing a bug that causes us to mistakenly demote a gen2 region to gen0 - #83341

Merged
mangod9 merged 1 commit into
release/7.0-stagingfrom
backport/pr-82413-to-release/7.0
Apr 4, 2023
Merged

[release/7.0] Fixing a bug that causes us to mistakenly demote a gen2 region to gen0#83341
mangod9 merged 1 commit into
release/7.0-stagingfrom
backport/pr-82413-to-release/7.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Mar 13, 2023

Copy link
Copy Markdown
Contributor

Backport of #82413 to release/7.0

/cc @mangod9@Maoni0

Customer Impact

An infinite loop in GC. Multiple customers have reported this issue and it happens when a gen0 region that's almost fully occupied (< 24 bytes free) with one giant plug (ie, no free objects at all in the region). This causes allocate_in_condemned_generations to go into an infinite loop because in ephemeral generations we expect short plugs, ie, we should be able to allocate a min free object in front of each plug. And normally we can because when we allocate objects in gen0 we make sure to break up the allocation contexts with min free objects and when we compact into gen1 we form short plugs.

Reported by multiple customers in #80073. While validating we also found that the repro causes another failure which is fixed by #83342

Testing

local functional testing; GC stress testing; also validated by a customer who hit the issue.

Risk

low risk. this would cause a region that happened to be in gen0 by accident to now actually go through our demotion logic which we already do for all other regions anyway.

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

We are seeing a gen0 region that's almost fully occupied (< 24 bytes free) with one giant plug (ie, no free objects at all in the region).
This causes allocate_in_condemned_generations to go into an infinite loop because in ephemeral generations we expect short plugs,
ie, we should be able to allocate a min free object in front of each plug. And normally we can because when we allocate objects in gen0
we make sure to break up the allocation contexts with min free objects and when we compact into gen1 we form short plugs.
We are in this situation when all of the following conditions are true -
+ we did a gen2 compacting GC that generates a pinned plug in a gen2 region almost as big as the whole region. my guess for the reason why there's this giant pinned plug is because that gen2 region was already really compact so when we called allocate_in_condemned_generations on the non pinned plugs that are next to some pinned plugs in it we discovered we can't move the non pinned plugs anyway so we artificially pinned them and formed a giant pinned plug. and during this GC those objects were no longer pinned so we have one giant non pinned plug.
+ this gen2 region needs to be the last region with pinned plugs;
+ this gen2 region hasn't been consumed by allocate_in_condemned_generations yet so it was processed by process_remaining_regions;
Then in process_remaining_regions we'll set the plan_gen_num for that gen2 region to 0 because we are doing
set_region_plan_gen_num_sip (current_region, current_plan_gen_num);
instead of going through the demotion logic to decide whether we should demote this region or not.
@ghostghost added the area-GC-coreclr label Mar 13, 2023
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #82413 to release/7.0

/cc @mangod9@Maoni0

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author:github-actions[bot]
Assignees:-
Labels:

area-GC-coreclr

Milestone:-

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

approved. we will take for consideration in 7.0.x

@jeffschwMSFTjeffschwMSFT added the Servicing-approved Approved for servicing release label Mar 16, 2023
@jeffschwMSFTjeffschwMSFT added this to the 7.0.6 milestone Mar 16, 2023
@carlossanlop

Copy link
Copy Markdown
Contributor

I'm retargeting this PR to the new release/7.0-staging branch, which is the one that we will use from now on for servicing fixes.

Repo maintainers will now be allowed to merge their own servicing PR as long as it meets the requirements:

  • It is approved by Tactics (signaled by adding the Servicing-approved label).
  • It's signed-off by an area owner.
  • The CI is green, or the failures are investigated as unrelated.
  • And if the PR touches an OOB package, the necessary OOB authoring changes are added.

The new process is described here: runtime/docs/project/library-servicing.md.

The infra team will be actively monitoring servicing PRs to ensure all requirements are met and to help with any issues.

Let me know if you have any questions.

@carlossanlop
carlossanlop changed the base branch from release/7.0 to release/7.0-stagingMarch 28, 2023 20:57
@carlossanlopcarlossanlop added Servicing-approved Approved for servicing release and removed Servicing-approved Approved for servicing release labels Mar 30, 2023
@carlossanlop

Copy link
Copy Markdown
Contributor

Reminder: April 10th is the last day to merge backport PRs to ensure they get included in the May Release. PR owners are now in charge of merging their own PRs.

@mangod9
mangod9 merged commit f8777ff into release/7.0-stagingApr 4, 2023
@carlossanlop
carlossanlop deleted the backport/pr-82413-to-release/7.0 branch April 4, 2023 21:20
@ghostghost locked as resolved and limited conversation to collaborators May 5, 2023
@leecowleecow modified the milestones: 7.0.6, 7.0.7Jun 1, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-GC-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@carlossanlop@jeffschwMSFT@mangod9@leecow
, '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

[release/7.0] Fixing a bug that causes us to mistakenly demote a gen2 region to gen0 - #83341

Merged
mangod9 merged 1 commit into
release/7.0-stagingfrom
backport/pr-82413-to-release/7.0
Apr 4, 2023
Merged

[release/7.0] Fixing a bug that causes us to mistakenly demote a gen2 region to gen0#83341
mangod9 merged 1 commit into
release/7.0-stagingfrom
backport/pr-82413-to-release/7.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Mar 13, 2023

Copy link
Copy Markdown
Contributor

Backport of #82413 to release/7.0

/cc @mangod9@Maoni0

Customer Impact

An infinite loop in GC. Multiple customers have reported this issue and it happens when a gen0 region that's almost fully occupied (< 24 bytes free) with one giant plug (ie, no free objects at all in the region). This causes allocate_in_condemned_generations to go into an infinite loop because in ephemeral generations we expect short plugs, ie, we should be able to allocate a min free object in front of each plug. And normally we can because when we allocate objects in gen0 we make sure to break up the allocation contexts with min free objects and when we compact into gen1 we form short plugs.

Reported by multiple customers in #80073. While validating we also found that the repro causes another failure which is fixed by #83342

Testing

local functional testing; GC stress testing; also validated by a customer who hit the issue.

Risk

low risk. this would cause a region that happened to be in gen0 by accident to now actually go through our demotion logic which we already do for all other regions anyway.

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

We are seeing a gen0 region that's almost fully occupied (< 24 bytes free) with one giant plug (ie, no free objects at all in the region).
This causes allocate_in_condemned_generations to go into an infinite loop because in ephemeral generations we expect short plugs,
ie, we should be able to allocate a min free object in front of each plug. And normally we can because when we allocate objects in gen0
we make sure to break up the allocation contexts with min free objects and when we compact into gen1 we form short plugs.
We are in this situation when all of the following conditions are true -
+ we did a gen2 compacting GC that generates a pinned plug in a gen2 region almost as big as the whole region. my guess for the reason why there's this giant pinned plug is because that gen2 region was already really compact so when we called allocate_in_condemned_generations on the non pinned plugs that are next to some pinned plugs in it we discovered we can't move the non pinned plugs anyway so we artificially pinned them and formed a giant pinned plug. and during this GC those objects were no longer pinned so we have one giant non pinned plug.
+ this gen2 region needs to be the last region with pinned plugs;
+ this gen2 region hasn't been consumed by allocate_in_condemned_generations yet so it was processed by process_remaining_regions;
Then in process_remaining_regions we'll set the plan_gen_num for that gen2 region to 0 because we are doing
set_region_plan_gen_num_sip (current_region, current_plan_gen_num);
instead of going through the demotion logic to decide whether we should demote this region or not.
@ghostghost added the area-GC-coreclr label Mar 13, 2023
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #82413 to release/7.0

/cc @mangod9@Maoni0

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author:github-actions[bot]
Assignees:-
Labels:

area-GC-coreclr

Milestone:-

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

approved. we will take for consideration in 7.0.x

@jeffschwMSFTjeffschwMSFT added the Servicing-approved Approved for servicing release label Mar 16, 2023
@jeffschwMSFTjeffschwMSFT added this to the 7.0.6 milestone Mar 16, 2023
@carlossanlop

Copy link
Copy Markdown
Contributor

I'm retargeting this PR to the new release/7.0-staging branch, which is the one that we will use from now on for servicing fixes.

Repo maintainers will now be allowed to merge their own servicing PR as long as it meets the requirements:

  • It is approved by Tactics (signaled by adding the Servicing-approved label).
  • It's signed-off by an area owner.
  • The CI is green, or the failures are investigated as unrelated.
  • And if the PR touches an OOB package, the necessary OOB authoring changes are added.

The new process is described here: runtime/docs/project/library-servicing.md.

The infra team will be actively monitoring servicing PRs to ensure all requirements are met and to help with any issues.

Let me know if you have any questions.

@carlossanlop
carlossanlop changed the base branch from release/7.0 to release/7.0-stagingMarch 28, 2023 20:57
@carlossanlopcarlossanlop added Servicing-approved Approved for servicing release and removed Servicing-approved Approved for servicing release labels Mar 30, 2023
@carlossanlop

Copy link
Copy Markdown
Contributor

Reminder: April 10th is the last day to merge backport PRs to ensure they get included in the May Release. PR owners are now in charge of merging their own PRs.

@mangod9
mangod9 merged commit f8777ff into release/7.0-stagingApr 4, 2023
@carlossanlop
carlossanlop deleted the backport/pr-82413-to-release/7.0 branch April 4, 2023 21:20
@ghostghost locked as resolved and limited conversation to collaborators May 5, 2023
@leecowleecow modified the milestones: 7.0.6, 7.0.7Jun 1, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-GC-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@carlossanlop@jeffschwMSFT@mangod9@leecow
, '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

[release/7.0] Fixing a bug that causes us to mistakenly demote a gen2 region to gen0 - #83341

Merged
mangod9 merged 1 commit into
release/7.0-stagingfrom
backport/pr-82413-to-release/7.0
Apr 4, 2023
Merged

[release/7.0] Fixing a bug that causes us to mistakenly demote a gen2 region to gen0#83341
mangod9 merged 1 commit into
release/7.0-stagingfrom
backport/pr-82413-to-release/7.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Mar 13, 2023

Copy link
Copy Markdown
Contributor

Backport of #82413 to release/7.0

/cc @mangod9@Maoni0

Customer Impact

An infinite loop in GC. Multiple customers have reported this issue and it happens when a gen0 region that's almost fully occupied (< 24 bytes free) with one giant plug (ie, no free objects at all in the region). This causes allocate_in_condemned_generations to go into an infinite loop because in ephemeral generations we expect short plugs, ie, we should be able to allocate a min free object in front of each plug. And normally we can because when we allocate objects in gen0 we make sure to break up the allocation contexts with min free objects and when we compact into gen1 we form short plugs.

Reported by multiple customers in #80073. While validating we also found that the repro causes another failure which is fixed by #83342

Testing

local functional testing; GC stress testing; also validated by a customer who hit the issue.

Risk

low risk. this would cause a region that happened to be in gen0 by accident to now actually go through our demotion logic which we already do for all other regions anyway.

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

We are seeing a gen0 region that's almost fully occupied (< 24 bytes free) with one giant plug (ie, no free objects at all in the region).
This causes allocate_in_condemned_generations to go into an infinite loop because in ephemeral generations we expect short plugs,
ie, we should be able to allocate a min free object in front of each plug. And normally we can because when we allocate objects in gen0
we make sure to break up the allocation contexts with min free objects and when we compact into gen1 we form short plugs.
We are in this situation when all of the following conditions are true -
+ we did a gen2 compacting GC that generates a pinned plug in a gen2 region almost as big as the whole region. my guess for the reason why there's this giant pinned plug is because that gen2 region was already really compact so when we called allocate_in_condemned_generations on the non pinned plugs that are next to some pinned plugs in it we discovered we can't move the non pinned plugs anyway so we artificially pinned them and formed a giant pinned plug. and during this GC those objects were no longer pinned so we have one giant non pinned plug.
+ this gen2 region needs to be the last region with pinned plugs;
+ this gen2 region hasn't been consumed by allocate_in_condemned_generations yet so it was processed by process_remaining_regions;
Then in process_remaining_regions we'll set the plan_gen_num for that gen2 region to 0 because we are doing
set_region_plan_gen_num_sip (current_region, current_plan_gen_num);
instead of going through the demotion logic to decide whether we should demote this region or not.
@ghostghost added the area-GC-coreclr label Mar 13, 2023
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #82413 to release/7.0

/cc @mangod9@Maoni0

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author:github-actions[bot]
Assignees:-
Labels:

area-GC-coreclr

Milestone:-

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

approved. we will take for consideration in 7.0.x

@jeffschwMSFTjeffschwMSFT added the Servicing-approved Approved for servicing release label Mar 16, 2023
@jeffschwMSFTjeffschwMSFT added this to the 7.0.6 milestone Mar 16, 2023
@carlossanlop

Copy link
Copy Markdown
Contributor

I'm retargeting this PR to the new release/7.0-staging branch, which is the one that we will use from now on for servicing fixes.

Repo maintainers will now be allowed to merge their own servicing PR as long as it meets the requirements:

  • It is approved by Tactics (signaled by adding the Servicing-approved label).
  • It's signed-off by an area owner.
  • The CI is green, or the failures are investigated as unrelated.
  • And if the PR touches an OOB package, the necessary OOB authoring changes are added.

The new process is described here: runtime/docs/project/library-servicing.md.

The infra team will be actively monitoring servicing PRs to ensure all requirements are met and to help with any issues.

Let me know if you have any questions.

@carlossanlop
carlossanlop changed the base branch from release/7.0 to release/7.0-stagingMarch 28, 2023 20:57
@carlossanlopcarlossanlop added Servicing-approved Approved for servicing release and removed Servicing-approved Approved for servicing release labels Mar 30, 2023
@carlossanlop

Copy link
Copy Markdown
Contributor

Reminder: April 10th is the last day to merge backport PRs to ensure they get included in the May Release. PR owners are now in charge of merging their own PRs.

@mangod9
mangod9 merged commit f8777ff into release/7.0-stagingApr 4, 2023
@carlossanlop
carlossanlop deleted the backport/pr-82413-to-release/7.0 branch April 4, 2023 21:20
@ghostghost locked as resolved and limited conversation to collaborators May 5, 2023
@leecowleecow modified the milestones: 7.0.6, 7.0.7Jun 1, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-GC-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@carlossanlop@jeffschwMSFT@mangod9@leecow
, '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

[release/7.0] Fixing a bug that causes us to mistakenly demote a gen2 region to gen0 - #83341

Merged
mangod9 merged 1 commit into
release/7.0-stagingfrom
backport/pr-82413-to-release/7.0
Apr 4, 2023
Merged

[release/7.0] Fixing a bug that causes us to mistakenly demote a gen2 region to gen0#83341
mangod9 merged 1 commit into
release/7.0-stagingfrom
backport/pr-82413-to-release/7.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Mar 13, 2023

Copy link
Copy Markdown
Contributor

Backport of #82413 to release/7.0

/cc @mangod9@Maoni0

Customer Impact

An infinite loop in GC. Multiple customers have reported this issue and it happens when a gen0 region that's almost fully occupied (< 24 bytes free) with one giant plug (ie, no free objects at all in the region). This causes allocate_in_condemned_generations to go into an infinite loop because in ephemeral generations we expect short plugs, ie, we should be able to allocate a min free object in front of each plug. And normally we can because when we allocate objects in gen0 we make sure to break up the allocation contexts with min free objects and when we compact into gen1 we form short plugs.

Reported by multiple customers in #80073. While validating we also found that the repro causes another failure which is fixed by #83342

Testing

local functional testing; GC stress testing; also validated by a customer who hit the issue.

Risk

low risk. this would cause a region that happened to be in gen0 by accident to now actually go through our demotion logic which we already do for all other regions anyway.

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

We are seeing a gen0 region that's almost fully occupied (< 24 bytes free) with one giant plug (ie, no free objects at all in the region).
This causes allocate_in_condemned_generations to go into an infinite loop because in ephemeral generations we expect short plugs,
ie, we should be able to allocate a min free object in front of each plug. And normally we can because when we allocate objects in gen0
we make sure to break up the allocation contexts with min free objects and when we compact into gen1 we form short plugs.
We are in this situation when all of the following conditions are true -
+ we did a gen2 compacting GC that generates a pinned plug in a gen2 region almost as big as the whole region. my guess for the reason why there's this giant pinned plug is because that gen2 region was already really compact so when we called allocate_in_condemned_generations on the non pinned plugs that are next to some pinned plugs in it we discovered we can't move the non pinned plugs anyway so we artificially pinned them and formed a giant pinned plug. and during this GC those objects were no longer pinned so we have one giant non pinned plug.
+ this gen2 region needs to be the last region with pinned plugs;
+ this gen2 region hasn't been consumed by allocate_in_condemned_generations yet so it was processed by process_remaining_regions;
Then in process_remaining_regions we'll set the plan_gen_num for that gen2 region to 0 because we are doing
set_region_plan_gen_num_sip (current_region, current_plan_gen_num);
instead of going through the demotion logic to decide whether we should demote this region or not.
@ghostghost added the area-GC-coreclr label Mar 13, 2023
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #82413 to release/7.0

/cc @mangod9@Maoni0

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author:github-actions[bot]
Assignees:-
Labels:

area-GC-coreclr

Milestone:-

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

approved. we will take for consideration in 7.0.x

@jeffschwMSFTjeffschwMSFT added the Servicing-approved Approved for servicing release label Mar 16, 2023
@jeffschwMSFTjeffschwMSFT added this to the 7.0.6 milestone Mar 16, 2023
@carlossanlop

Copy link
Copy Markdown
Contributor

I'm retargeting this PR to the new release/7.0-staging branch, which is the one that we will use from now on for servicing fixes.

Repo maintainers will now be allowed to merge their own servicing PR as long as it meets the requirements:

  • It is approved by Tactics (signaled by adding the Servicing-approved label).
  • It's signed-off by an area owner.
  • The CI is green, or the failures are investigated as unrelated.
  • And if the PR touches an OOB package, the necessary OOB authoring changes are added.

The new process is described here: runtime/docs/project/library-servicing.md.

The infra team will be actively monitoring servicing PRs to ensure all requirements are met and to help with any issues.

Let me know if you have any questions.

@carlossanlop
carlossanlop changed the base branch from release/7.0 to release/7.0-stagingMarch 28, 2023 20:57
@carlossanlopcarlossanlop added Servicing-approved Approved for servicing release and removed Servicing-approved Approved for servicing release labels Mar 30, 2023
@carlossanlop

Copy link
Copy Markdown
Contributor

Reminder: April 10th is the last day to merge backport PRs to ensure they get included in the May Release. PR owners are now in charge of merging their own PRs.

@mangod9
mangod9 merged commit f8777ff into release/7.0-stagingApr 4, 2023
@carlossanlop
carlossanlop deleted the backport/pr-82413-to-release/7.0 branch April 4, 2023 21:20
@ghostghost locked as resolved and limited conversation to collaborators May 5, 2023
@leecowleecow modified the milestones: 7.0.6, 7.0.7Jun 1, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-GC-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@carlossanlop@jeffschwMSFT@mangod9@leecow
, '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

[release/7.0] Fixing a bug that causes us to mistakenly demote a gen2 region to gen0 - #83341

Merged
mangod9 merged 1 commit into
release/7.0-stagingfrom
backport/pr-82413-to-release/7.0
Apr 4, 2023
Merged

[release/7.0] Fixing a bug that causes us to mistakenly demote a gen2 region to gen0#83341
mangod9 merged 1 commit into
release/7.0-stagingfrom
backport/pr-82413-to-release/7.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Mar 13, 2023

Copy link
Copy Markdown
Contributor

Backport of #82413 to release/7.0

/cc @mangod9@Maoni0

Customer Impact

An infinite loop in GC. Multiple customers have reported this issue and it happens when a gen0 region that's almost fully occupied (< 24 bytes free) with one giant plug (ie, no free objects at all in the region). This causes allocate_in_condemned_generations to go into an infinite loop because in ephemeral generations we expect short plugs, ie, we should be able to allocate a min free object in front of each plug. And normally we can because when we allocate objects in gen0 we make sure to break up the allocation contexts with min free objects and when we compact into gen1 we form short plugs.

Reported by multiple customers in #80073. While validating we also found that the repro causes another failure which is fixed by #83342

Testing

local functional testing; GC stress testing; also validated by a customer who hit the issue.

Risk

low risk. this would cause a region that happened to be in gen0 by accident to now actually go through our demotion logic which we already do for all other regions anyway.

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

We are seeing a gen0 region that's almost fully occupied (< 24 bytes free) with one giant plug (ie, no free objects at all in the region).
This causes allocate_in_condemned_generations to go into an infinite loop because in ephemeral generations we expect short plugs,
ie, we should be able to allocate a min free object in front of each plug. And normally we can because when we allocate objects in gen0
we make sure to break up the allocation contexts with min free objects and when we compact into gen1 we form short plugs.
We are in this situation when all of the following conditions are true -
+ we did a gen2 compacting GC that generates a pinned plug in a gen2 region almost as big as the whole region. my guess for the reason why there's this giant pinned plug is because that gen2 region was already really compact so when we called allocate_in_condemned_generations on the non pinned plugs that are next to some pinned plugs in it we discovered we can't move the non pinned plugs anyway so we artificially pinned them and formed a giant pinned plug. and during this GC those objects were no longer pinned so we have one giant non pinned plug.
+ this gen2 region needs to be the last region with pinned plugs;
+ this gen2 region hasn't been consumed by allocate_in_condemned_generations yet so it was processed by process_remaining_regions;
Then in process_remaining_regions we'll set the plan_gen_num for that gen2 region to 0 because we are doing
set_region_plan_gen_num_sip (current_region, current_plan_gen_num);
instead of going through the demotion logic to decide whether we should demote this region or not.
@ghostghost added the area-GC-coreclr label Mar 13, 2023
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #82413 to release/7.0

/cc @mangod9@Maoni0

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author:github-actions[bot]
Assignees:-
Labels:

area-GC-coreclr

Milestone:-

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

approved. we will take for consideration in 7.0.x

@jeffschwMSFTjeffschwMSFT added the Servicing-approved Approved for servicing release label Mar 16, 2023
@jeffschwMSFTjeffschwMSFT added this to the 7.0.6 milestone Mar 16, 2023
@carlossanlop

Copy link
Copy Markdown
Contributor

I'm retargeting this PR to the new release/7.0-staging branch, which is the one that we will use from now on for servicing fixes.

Repo maintainers will now be allowed to merge their own servicing PR as long as it meets the requirements:

  • It is approved by Tactics (signaled by adding the Servicing-approved label).
  • It's signed-off by an area owner.
  • The CI is green, or the failures are investigated as unrelated.
  • And if the PR touches an OOB package, the necessary OOB authoring changes are added.

The new process is described here: runtime/docs/project/library-servicing.md.

The infra team will be actively monitoring servicing PRs to ensure all requirements are met and to help with any issues.

Let me know if you have any questions.

@carlossanlop
carlossanlop changed the base branch from release/7.0 to release/7.0-stagingMarch 28, 2023 20:57
@carlossanlopcarlossanlop added Servicing-approved Approved for servicing release and removed Servicing-approved Approved for servicing release labels Mar 30, 2023
@carlossanlop

Copy link
Copy Markdown
Contributor

Reminder: April 10th is the last day to merge backport PRs to ensure they get included in the May Release. PR owners are now in charge of merging their own PRs.

@mangod9
mangod9 merged commit f8777ff into release/7.0-stagingApr 4, 2023
@carlossanlop
carlossanlop deleted the backport/pr-82413-to-release/7.0 branch April 4, 2023 21:20
@ghostghost locked as resolved and limited conversation to collaborators May 5, 2023
@leecowleecow modified the milestones: 7.0.6, 7.0.7Jun 1, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-GC-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@carlossanlop@jeffschwMSFT@mangod9@leecow
, '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

[release/7.0] Fixing a bug that causes us to mistakenly demote a gen2 region to gen0 - #83341

Merged
mangod9 merged 1 commit into
release/7.0-stagingfrom
backport/pr-82413-to-release/7.0
Apr 4, 2023
Merged

[release/7.0] Fixing a bug that causes us to mistakenly demote a gen2 region to gen0#83341
mangod9 merged 1 commit into
release/7.0-stagingfrom
backport/pr-82413-to-release/7.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Mar 13, 2023

Copy link
Copy Markdown
Contributor

Backport of #82413 to release/7.0

/cc @mangod9@Maoni0

Customer Impact

An infinite loop in GC. Multiple customers have reported this issue and it happens when a gen0 region that's almost fully occupied (< 24 bytes free) with one giant plug (ie, no free objects at all in the region). This causes allocate_in_condemned_generations to go into an infinite loop because in ephemeral generations we expect short plugs, ie, we should be able to allocate a min free object in front of each plug. And normally we can because when we allocate objects in gen0 we make sure to break up the allocation contexts with min free objects and when we compact into gen1 we form short plugs.

Reported by multiple customers in #80073. While validating we also found that the repro causes another failure which is fixed by #83342

Testing

local functional testing; GC stress testing; also validated by a customer who hit the issue.

Risk

low risk. this would cause a region that happened to be in gen0 by accident to now actually go through our demotion logic which we already do for all other regions anyway.

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

We are seeing a gen0 region that's almost fully occupied (< 24 bytes free) with one giant plug (ie, no free objects at all in the region).
This causes allocate_in_condemned_generations to go into an infinite loop because in ephemeral generations we expect short plugs,
ie, we should be able to allocate a min free object in front of each plug. And normally we can because when we allocate objects in gen0
we make sure to break up the allocation contexts with min free objects and when we compact into gen1 we form short plugs.
We are in this situation when all of the following conditions are true -
+ we did a gen2 compacting GC that generates a pinned plug in a gen2 region almost as big as the whole region. my guess for the reason why there's this giant pinned plug is because that gen2 region was already really compact so when we called allocate_in_condemned_generations on the non pinned plugs that are next to some pinned plugs in it we discovered we can't move the non pinned plugs anyway so we artificially pinned them and formed a giant pinned plug. and during this GC those objects were no longer pinned so we have one giant non pinned plug.
+ this gen2 region needs to be the last region with pinned plugs;
+ this gen2 region hasn't been consumed by allocate_in_condemned_generations yet so it was processed by process_remaining_regions;
Then in process_remaining_regions we'll set the plan_gen_num for that gen2 region to 0 because we are doing
set_region_plan_gen_num_sip (current_region, current_plan_gen_num);
instead of going through the demotion logic to decide whether we should demote this region or not.
@ghostghost added the area-GC-coreclr label Mar 13, 2023
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #82413 to release/7.0

/cc @mangod9@Maoni0

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author:github-actions[bot]
Assignees:-
Labels:

area-GC-coreclr

Milestone:-

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

approved. we will take for consideration in 7.0.x

@jeffschwMSFTjeffschwMSFT added the Servicing-approved Approved for servicing release label Mar 16, 2023
@jeffschwMSFTjeffschwMSFT added this to the 7.0.6 milestone Mar 16, 2023
@carlossanlop

Copy link
Copy Markdown
Contributor

I'm retargeting this PR to the new release/7.0-staging branch, which is the one that we will use from now on for servicing fixes.

Repo maintainers will now be allowed to merge their own servicing PR as long as it meets the requirements:

  • It is approved by Tactics (signaled by adding the Servicing-approved label).
  • It's signed-off by an area owner.
  • The CI is green, or the failures are investigated as unrelated.
  • And if the PR touches an OOB package, the necessary OOB authoring changes are added.

The new process is described here: runtime/docs/project/library-servicing.md.

The infra team will be actively monitoring servicing PRs to ensure all requirements are met and to help with any issues.

Let me know if you have any questions.

@carlossanlop
carlossanlop changed the base branch from release/7.0 to release/7.0-stagingMarch 28, 2023 20:57
@carlossanlopcarlossanlop added Servicing-approved Approved for servicing release and removed Servicing-approved Approved for servicing release labels Mar 30, 2023
@carlossanlop

Copy link
Copy Markdown
Contributor

Reminder: April 10th is the last day to merge backport PRs to ensure they get included in the May Release. PR owners are now in charge of merging their own PRs.

@mangod9
mangod9 merged commit f8777ff into release/7.0-stagingApr 4, 2023
@carlossanlop
carlossanlop deleted the backport/pr-82413-to-release/7.0 branch April 4, 2023 21:20
@ghostghost locked as resolved and limited conversation to collaborators May 5, 2023
@leecowleecow modified the milestones: 7.0.6, 7.0.7Jun 1, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-GC-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@carlossanlop@jeffschwMSFT@mangod9@leecow
, '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

[release/7.0] Fixing a bug that causes us to mistakenly demote a gen2 region to gen0 - #83341

Merged
mangod9 merged 1 commit into
release/7.0-stagingfrom
backport/pr-82413-to-release/7.0
Apr 4, 2023
Merged

[release/7.0] Fixing a bug that causes us to mistakenly demote a gen2 region to gen0#83341
mangod9 merged 1 commit into
release/7.0-stagingfrom
backport/pr-82413-to-release/7.0

Conversation

@github-actions

@github-actionsgithub-actionsBot commented Mar 13, 2023

Copy link
Copy Markdown
Contributor

Backport of #82413 to release/7.0

/cc @mangod9@Maoni0

Customer Impact

An infinite loop in GC. Multiple customers have reported this issue and it happens when a gen0 region that's almost fully occupied (< 24 bytes free) with one giant plug (ie, no free objects at all in the region). This causes allocate_in_condemned_generations to go into an infinite loop because in ephemeral generations we expect short plugs, ie, we should be able to allocate a min free object in front of each plug. And normally we can because when we allocate objects in gen0 we make sure to break up the allocation contexts with min free objects and when we compact into gen1 we form short plugs.

Reported by multiple customers in #80073. While validating we also found that the repro causes another failure which is fixed by #83342

Testing

local functional testing; GC stress testing; also validated by a customer who hit the issue.

Risk

low risk. this would cause a region that happened to be in gen0 by accident to now actually go through our demotion logic which we already do for all other regions anyway.

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

We are seeing a gen0 region that's almost fully occupied (< 24 bytes free) with one giant plug (ie, no free objects at all in the region).
This causes allocate_in_condemned_generations to go into an infinite loop because in ephemeral generations we expect short plugs,
ie, we should be able to allocate a min free object in front of each plug. And normally we can because when we allocate objects in gen0
we make sure to break up the allocation contexts with min free objects and when we compact into gen1 we form short plugs.
We are in this situation when all of the following conditions are true -
+ we did a gen2 compacting GC that generates a pinned plug in a gen2 region almost as big as the whole region. my guess for the reason why there's this giant pinned plug is because that gen2 region was already really compact so when we called allocate_in_condemned_generations on the non pinned plugs that are next to some pinned plugs in it we discovered we can't move the non pinned plugs anyway so we artificially pinned them and formed a giant pinned plug. and during this GC those objects were no longer pinned so we have one giant non pinned plug.
+ this gen2 region needs to be the last region with pinned plugs;
+ this gen2 region hasn't been consumed by allocate_in_condemned_generations yet so it was processed by process_remaining_regions;
Then in process_remaining_regions we'll set the plan_gen_num for that gen2 region to 0 because we are doing
set_region_plan_gen_num_sip (current_region, current_plan_gen_num);
instead of going through the demotion logic to decide whether we should demote this region or not.
@ghostghost added the area-GC-coreclr label Mar 13, 2023
@ghost

Copy link
Copy Markdown

Tagging subscribers to this area: @dotnet/gc
See info in area-owners.md if you want to be subscribed.

Issue Details

Backport of #82413 to release/7.0

/cc @mangod9@Maoni0

Customer Impact

Testing

Risk

IMPORTANT: Is this backport for a servicing release? If so and this change touches code that ships in a NuGet package, please make certain that you have added any necessary package authoring and gotten it explicitly reviewed.

Author:github-actions[bot]
Assignees:-
Labels:

area-GC-coreclr

Milestone:-

@jeffschwMSFTjeffschwMSFT left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

approved. we will take for consideration in 7.0.x

@jeffschwMSFTjeffschwMSFT added the Servicing-approved Approved for servicing release label Mar 16, 2023
@jeffschwMSFTjeffschwMSFT added this to the 7.0.6 milestone Mar 16, 2023
@carlossanlop

Copy link
Copy Markdown
Contributor

I'm retargeting this PR to the new release/7.0-staging branch, which is the one that we will use from now on for servicing fixes.

Repo maintainers will now be allowed to merge their own servicing PR as long as it meets the requirements:

  • It is approved by Tactics (signaled by adding the Servicing-approved label).
  • It's signed-off by an area owner.
  • The CI is green, or the failures are investigated as unrelated.
  • And if the PR touches an OOB package, the necessary OOB authoring changes are added.

The new process is described here: runtime/docs/project/library-servicing.md.

The infra team will be actively monitoring servicing PRs to ensure all requirements are met and to help with any issues.

Let me know if you have any questions.

@carlossanlop
carlossanlop changed the base branch from release/7.0 to release/7.0-stagingMarch 28, 2023 20:57
@carlossanlopcarlossanlop added Servicing-approved Approved for servicing release and removed Servicing-approved Approved for servicing release labels Mar 30, 2023
@carlossanlop

Copy link
Copy Markdown
Contributor

Reminder: April 10th is the last day to merge backport PRs to ensure they get included in the May Release. PR owners are now in charge of merging their own PRs.

@mangod9
mangod9 merged commit f8777ff into release/7.0-stagingApr 4, 2023
@carlossanlop
carlossanlop deleted the backport/pr-82413-to-release/7.0 branch April 4, 2023 21:20
@ghostghost locked as resolved and limited conversation to collaborators May 5, 2023
@leecowleecow modified the milestones: 7.0.6, 7.0.7Jun 1, 2023
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-GC-coreclrServicing-approvedApproved for servicing release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@carlossanlop@jeffschwMSFT@mangod9@leecow