Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[automated] Merge branch 'release/8.0-rc2' => 'release/8.0' #92325

Conversation

dotnet-maestro-bot
Copy link
Contributor

I detected changes in the release/8.0-rc2 branch which have not been merged yet to release/8.0. I'm a robot and am configured to help you automatically keep release/8.0 up to date, so I've opened this PR.

This PR merges commits made on release/8.0-rc2 by the following committers:

  • github-actions[bot]
  • dotnet-maestro[bot]

Instructions for merging from UI

This PR will not be auto-merged. When pull request checks pass, complete this PR by creating a merge commit, not a squash or rebase commit.

merge button instructions

If this repo does not allow creating merge commits from the GitHub UI, use command line instructions.

Instructions for merging via command line

Run these commands to merge this pull request from the command line.

git fetch
git checkout release/8.0-rc2
git pull --ff-only
git checkout release/8.0
git pull --ff-only
git merge --no-ff release/8.0-rc2

# If there are merge conflicts, resolve them and then run git merge --continue to complete the merge
# Pushing the changes to the PR branch will re-trigger PR validation.
git push https://github.com/dotnet-maestro-bot/runtime HEAD:merge/release/8.0-rc2-to-release/8.0
or if you are using SSH
git push [email protected]:dotnet-maestro-bot/runtime HEAD:merge/release/8.0-rc2-to-release/8.0

After PR checks are complete push the branch

git push

Instructions for resolving conflicts

⚠️ If there are merge conflicts, you will need to resolve them manually before merging. You can do this using GitHub or using the command line.

Instructions for updating this pull request

Contributors to this repo have permission update this pull request by pushing to the branch 'merge/release/8.0-rc2-to-release/8.0'. This can be done to resolve conflicts or make other changes to this pull request before it is merged.

git checkout -b merge/release/8.0-rc2-to-release/8.0 release/8.0
git pull https://github.com/dotnet-maestro-bot/runtime merge/release/8.0-rc2-to-release/8.0
(make changes)
git commit -m "Updated PR with my changes"
git push https://github.com/dotnet-maestro-bot/runtime HEAD:merge/release/8.0-rc2-to-release/8.0
or if you are using SSH
git checkout -b merge/release/8.0-rc2-to-release/8.0 release/8.0
git pull [email protected]:dotnet-maestro-bot/runtime merge/release/8.0-rc2-to-release/8.0
(make changes)
git commit -m "Updated PR with my changes"
git push [email protected]:dotnet-maestro-bot/runtime HEAD:merge/release/8.0-rc2-to-release/8.0

Contact .NET Core Engineering if you have questions or issues.
Also, if this PR was generated incorrectly, help us fix it. See https://github.com/dotnet/arcade/blob/master/scripts/GitHubMergeBranches.ps1.

dotnet-maestro bot and others added 4 commits September 19, 2023 17:12
…19.1 (dotnet#92306)

Microsoft.NET.Workload.Emscripten.Current.Manifest-8.0.100.Transport
 From Version 8.0.0-rtm.23469.3 -> To Version 8.0.0-rc.2.23469.1

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Tarek Mahmoud Sayed <[email protected]>
Co-authored-by: Carlos Sánchez López <[email protected]>
Handle the case where we're indirectly updating a local with a value
that is not a constant.

Fixes dotnet#92218.

Co-authored-by: Andy Ayers <[email protected]>
Co-authored-by: Carlos Sánchez López <[email protected]>
…semantics (dotnet#92308)

Co-authored-by: Tanner Gooding <[email protected]>
Co-authored-by: Carlos Sánchez López <[email protected]>
@ghost
Copy link

ghost commented Sep 20, 2023

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

Issue Details

I detected changes in the release/8.0-rc2 branch which have not been merged yet to release/8.0. I'm a robot and am configured to help you automatically keep release/8.0 up to date, so I've opened this PR.

This PR merges commits made on release/8.0-rc2 by the following committers:

  • github-actions[bot]
  • dotnet-maestro[bot]

Instructions for merging from UI

This PR will not be auto-merged. When pull request checks pass, complete this PR by creating a merge commit, not a squash or rebase commit.

merge button instructions

If this repo does not allow creating merge commits from the GitHub UI, use command line instructions.

Instructions for merging via command line

Run these commands to merge this pull request from the command line.

git fetch
git checkout release/8.0-rc2
git pull --ff-only
git checkout release/8.0
git pull --ff-only
git merge --no-ff release/8.0-rc2

# If there are merge conflicts, resolve them and then run git merge --continue to complete the merge
# Pushing the changes to the PR branch will re-trigger PR validation.
git push https://github.com/dotnet-maestro-bot/runtime HEAD:merge/release/8.0-rc2-to-release/8.0
or if you are using SSH
git push [email protected]:dotnet-maestro-bot/runtime HEAD:merge/release/8.0-rc2-to-release/8.0

After PR checks are complete push the branch

git push

Instructions for resolving conflicts

⚠️ If there are merge conflicts, you will need to resolve them manually before merging. You can do this using GitHub or using the command line.

Instructions for updating this pull request

Contributors to this repo have permission update this pull request by pushing to the branch 'merge/release/8.0-rc2-to-release/8.0'. This can be done to resolve conflicts or make other changes to this pull request before it is merged.

git checkout -b merge/release/8.0-rc2-to-release/8.0 release/8.0
git pull https://github.com/dotnet-maestro-bot/runtime merge/release/8.0-rc2-to-release/8.0
(make changes)
git commit -m "Updated PR with my changes"
git push https://github.com/dotnet-maestro-bot/runtime HEAD:merge/release/8.0-rc2-to-release/8.0
or if you are using SSH
git checkout -b merge/release/8.0-rc2-to-release/8.0 release/8.0
git pull [email protected]:dotnet-maestro-bot/runtime merge/release/8.0-rc2-to-release/8.0
(make changes)
git commit -m "Updated PR with my changes"
git push [email protected]:dotnet-maestro-bot/runtime HEAD:merge/release/8.0-rc2-to-release/8.0

Contact .NET Core Engineering if you have questions or issues.
Also, if this PR was generated incorrectly, help us fix it. See https://github.com/dotnet/arcade/blob/master/scripts/GitHubMergeBranches.ps1.

Author: dotnet-maestro-bot
Assignees: -
Labels:

area-Infrastructure-libraries

Milestone: -

Copy link
Member

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

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

Merge commit.

@carlossanlop carlossanlop added Servicing-approved Approved for servicing release area-codeflow for labeling automated codeflow and removed area-Infrastructure-libraries labels Sep 20, 2023
@carlossanlop carlossanlop added this to the 8.0.0 milestone Sep 20, 2023
Maoni0 and others added 3 commits September 20, 2023 09:25
* new synchronization mechanism for DATAS (dotnet#90726)

The current mechanism has a fundamental flaw which is the idling threads can start running at unpredictable times when they are woken up. This causes all sorts of problems. For example, when a thread gets here in gc_thread_function -

`if (n_heaps <= heap_number)`

if it's true it's supposed to wait. But its execution could be delayed so after it reads n_heaps it can stop for a while since no thread is waiting on this thread anyway... till some time later when a heap count changes happens again and it requires this thread to participating. And now this thread does the comparison and discovers that it needs to wait so it goes idle and all other threads will just be waiting for this thread to join.

Another example is it's not safe to change the heap count for a join from a larger one to a smaller one. It's fine to change from a smaller one to a larger one because all the threads participating will have to run in order for a join to finish. But if no one is waiting on a thread, it could just wake up from the event being set by the last thread joining and not run for a while. Then go back to the respin loop at a point where the color was changed and changed again! So now it thinks it can proceed with a join it does not belong to. And of course that wouldn't work.

The way threads are going idle/waking up is hard to keep track of - not only does it involve the gc_start_event and gc_idle_thread_event, it also uses WaitForGCEvent which is used by SuspendEE/RestartEE which in turn means whenever we want to call these we'd need to care about how that would affect this.

The new mechanism only uses gc_start_event and gc_idle_thread_event, but I changed gc_idle_thread_event to a per heap event. We can easily track which threads are going idling easily - whenever a thread is about to wait on the idle event, we increase the current idle_thread_count. And when we increase the heap count we only set the gc_idle_thread_event for the new heaps that are about to participate so we can deduct that many from idle_thread_count. There's a much simpler code path between "we know we don't need these threads anymore" to "these threads are at a known point" because the next time gc_start_event is set (ie, a GC is requested) we make sure to get these threads to a good known point, ie, we wait till all of them have completed increasing idle_thread_count.

Also fixed a couple of other problems that I hit while testing the new mechanism -

We are setting freeable_uoh_segment and freeable_soh_segment in decommission_heap to DECOMMISSIONED_REGION_P. And this causes us to simply lose the value for them. We should make sure we do push these to the free regions before we start changing the heap count.

We should also call background_delay_delete_uoh_segments before we start changing the heap count so we can get rid of the regions marked with heap_segment_flags_uoh_delete. If we allow these to be rearranged in equalize_promoted_bytes it means the order can change the invariant of the first region never being deleted no longer holds true and we can AV in this method.

I added an new method delay_free_segments to perform both tasks.

The accounting of generation_free_list_space is slightly off for LOH which causes us to hit assert (gen_size >= dd_fragmentation (dd)); in change_heap_count because we were not counting the loh_pad size.
I also disabled assert (free_list_space_decrease <= dd_fragmentation (dd)); for gen2 since I'm seeing this fired while I'm doing stress runs. I have yet to investigate this since I didn't want to add yet more changes to this PR.

* fixed problems with how sampling is done and how we suspend to change heap count in DATAS (dotnet#91712)

+ Moved the sample recording into when we are suspended. The way we were calculating the throughput cost was in check_heap_count (which is called right after we restart EE on heap0), we record the msl_wait_time (and reset it to 0 for soh/uoh). This is not synchronized with the allocating threads (which are already running at this point). So what can happen is the allocating threads are already accumulated more wait time which is attributed to this GC but it's not within the period we are counting for this GC (and we lose this part for the next GC). For BGC this is incorrect. If an ephemeral GC did happen before the BGC starts, we'd be adding a sample for that GC which is basically correct for that eph GC. But if an eph GC did not happen, we are just adding a random sample which is calculating the tcp as (msl wait + whatever GC that was finished before this BGC) so obviously incorrect.

+ Added gen2 sampling - this was adapted from Peter's gen2 sampling changes. This serves as a backstop in case the existing sampling doesn't ever pick gen2 GC costs. I made the following fixes -

1) changed the way we calculated the median

2) moved where this is calculated to again avoid timing issues

3) made the gen2 samples actually count instead of losing that info if we happen to sample when a gen2 didn't just occur.

+ Changed when check_heap_count is called - the previous place is right after a suspension which does not help with spacing the suspension time out (it was "suspend for GC" then "immediately suspend to change heap count"). And it caused a problem with BGC which was it always tried to change heap count when it couldn't because BGC was in progress. I changed this to be on a timeout to intentionally space the suspensions out. Now most of the time, heap count changes happen due to this time out. If we are really in a situation where GCs are happening too quickly and we return from waiting on the ee_suspend_event due to a GC started, we change the heap count right before we do a GC. So this also helps with the BGC problem.

* gen0_bricks_cleared flag needs to be propagated when we change heap count (dotnet#90457)

when we change the heap count, in heap X we get a region from heap Y and the gen0_bricks_cleared flag from Y says false but heap X says true. So when we check the bricks on heap X, we assume it’s true but it’s not.

the fix is to detect if any heap has this flag as false and if so make all heaps’ flag false (tracking which region is moved from which other heap is something we need additional recording for and it’s not really worth doing just for this)

* a logging change

---------

Co-authored-by: Maoni0 <[email protected]>
…19.4 (dotnet#92322)

Microsoft.NET.Workload.Emscripten.Current.Manifest-8.0.100.Transport
 From Version 8.0.0-rc.2.23469.1 -> To Version 8.0.0-rc.2.23469.4

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
…he latest (dotnet#92353)

* [wasm] Add a dependency on dotnet/installer to get the latest

And update version from `.NET 8.0.1xx SDK` channel.

(cherry picked from commit c821c36)

* [wasm] WBT: Use --skip-sign-check when installing workloads

* [wasm] WBT: Update blazor tests to track changed path for Pages

.. from `Pages/` to `Components/Pages/`.
Picking the update from dotnet#92320 to
here, to avoid the overlap.
In case of a version like `8.0.100-rtm.23470.1`, we incorrectly
extracted `-rtm.23470` instead of `-rtm`, which resulted in trying to
install package named `Microsoft.NET.Workload.Emscripten.Current.Manifest-8.0.100-rtm.23470` instead of `Microsoft.NET.Workload.Emscripten.Current.Manifest-8.0.100-rtm`.
@radical
Copy link
Member

radical commented Sep 20, 2023

Added the emsdk update from #92320, and the corresponding fix, because 8.0 should be on -rtm and not -rc.2 versions.

@carlossanlop carlossanlop merged commit df1cf0c into dotnet:release/8.0 Sep 21, 2023
186 checks passed
@ghost ghost locked as resolved and limited conversation to collaborators Oct 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-codeflow for labeling automated codeflow Servicing-approved Approved for servicing release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants