Skip to content

Inline GC handle write barrier - #132245

Draft
jkotas with Copilot wants to merge 5 commits into
mainfrom
copilot/improve-gchandle-target-performance
Draft

Inline GC handle write barrier#132245
jkotas with Copilot wants to merge 5 commits into
mainfrom
copilot/improve-gchandle-target-performance

Conversation

Copilot AI commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

Validation

  • Built the CoreCLR GC native target successfully.
  • Built libraries successfully.
  • Full CoreCLR Release build reached the changed GC objects successfully but failed later on pre-existing JIT identifier errors in instrsxarch.h.
  • The focused GCHandle.Target test could not be built because test dependency assets have not been generated.

Performance

  • EgorBot measurement requested for Linux x64 and macOS ARM64.

Note

This pull request description was generated by AI/Copilot.

Copilot AI and others added 2 commits August 12, 2026 23:10
Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings August 12, 2026 23:22
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the CoreCLR GC handle write-barrier by moving the HndWriteBarrierWorker implementation into handletable.inl (so it can be inlined into its callers) and introduces a fast-path that avoids the handle-type lookup when the assigned target is in generation 0. This targets reduced overhead in hot handle assignment paths while keeping the GC/EE interface shape unchanged.

Changes:

  • Inline HndWriteBarrierWorker into src/coreclr/gc/handletable.inl and remove the out-of-line implementation from handletable.cpp.
  • Add an early-return fast-path in the worker to skip HandleFetchType(handle) when GetConvertedGeneration(value) is 0.
  • Move/adjust declarations (GetConvertedGeneration, HandleFetchType) to be available to the inlined implementation and update includes accordingly.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
src/coreclr/gc/handletablepriv.h Removes now-unneeded forward declarations previously used by the out-of-line worker.
src/coreclr/gc/handletable.inl Adds the inlined HndWriteBarrierWorker implementation and updates HndAssignHandle to call it directly.
src/coreclr/gc/handletable.h Adds handletableconstants.h include and exposes needed non-DAC prototypes for the inlined worker.
src/coreclr/gc/handletable.cpp Removes the out-of-line HndWriteBarrierWorker implementation (now provided inline).

Comment thread src/coreclr/gc/handletable.h Outdated
#ifndef DACCESS_COMPILE
int GetConvertedGeneration(_UNCHECKED_OBJECTREF obj);
uint32_t HandleFetchType(OBJECTHANDLE handle);
#endif // DACCESS_COMPILE

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.

@copilot Move these functions to handletable.inl‎. Also, make sure that they get inlined

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in a40af76 by moving GetConvertedGeneration and HandleFetchType into handletable.inl as FORCEINLINE helpers. Follow-up cleanup is in 86e3eb7.

Comment thread src/coreclr/gc/handletable.h Outdated
@@ -224,4 +229,3 @@ FORCEINLINE BOOL HndIsNullOrDestroyedHandle(_UNCHECKED_OBJECTREF value)
#include "handletable.inl"

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.

@copilot Delete #include "handletable.inl" in gcinternal.h - it is unnecessary since it is included here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removed in e4315e5. Note that handletable.h no longer includes handletable.inl (that include was moved to the .cpp files in a40af76): the inlined GetConvertedGeneration needs gc.h, and gc.h -> gchandletableimpl.h -> handletable.h is circular, so including the .inl from the header fails to compile. The .inl is included directly by the .cpp files that need it, and dropping it from gcinternal.h builds fine.

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 13, 2026 05:51
Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (1)

src/coreclr/gc/handletable.inl:68

  • The comment describing async pinned handle behavior has a few grammar/wording issues (e.g., missing space after //, “need”/“consider”). Updating it will make the intent clearer without changing behavior.
        //OverlappedData need special treatment: because all user data pointed by it needs to be reported by this handle,
        //its age is consider to be min age of the user data, to be simple, we just make it 0

Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Copilot AI review requested due to automatic review settings August 13, 2026 06:10

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.

Suppressed comments (2)

src/coreclr/gc/handletablepriv.h:171

  • HandleFetchType in handletable.inl reads the block type via raw uint8_t* indexing (segment[HANDLE_SEGMENT_BLOCK_TYPE_OFFSET + uBlock]). The existing offsetof static_assert guards the offset, but the implementation also assumes rgBlockType elements are 1 byte. Adding a size check here makes that assumption explicit and prevents silent breakage if the field type ever changes.
static_assert(offsetof(_TableSegmentHeader, rgBlockType) == HANDLE_SEGMENT_BLOCK_TYPE_OFFSET);

src/coreclr/gc/handletable.inl:43

  • New code in handletable.inl uses C-style casts when deriving segment pointers (e.g., (uint8_t*)((uintptr_t)handle & HANDLE_SEGMENT_ALIGN_MASK)), while nearby code already uses reinterpret_cast. This is easy to fix and aligns with the repo's native guidance to prefer C++ casts.
    uint8_t* barrier = (uint8_t*)((uintptr_t)handle & HANDLE_SEGMENT_ALIGN_MASK);
    _ASSERTE(barrier);

    uintptr_t offset = (uintptr_t)handle & HANDLE_SEGMENT_CONTENT_MASK;
    _ASSERTE(offset >= HANDLE_HEADER_SIZE);

Comment on lines 13 to 16
#include "softwarewritewatch.h"
#include "handletable.h"
#include "handletable.inl"
#include "gcenv.inl"
#include "gceventstatus.h"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants