[Foundation] Go back to the old implementation of NSObject.GetSuper ().#25405
[Foundation] Go back to the old implementation of NSObject.GetSuper ().#25405rolfbjarne wants to merge 6 commits into
Conversation
Use the old implementation of NSObject.GetSuper (): * Allocate native memory for it. * Cache that native memory in an NSObject field. * Free it when the NSObject is disposed. This is less performant, but any performance issues will be alleviated by the fact that NSObject.GetSuper () will be called much less after #25376. The great advantage is that NSObjectData is now smaller, and fits in the tagged memory returned by `ObjectiveCMarshal.CreateReferenceTrackingHandle`, which will simplify memory management a lot for CoreCLR. Contributes towards #25383.
There was a problem hiding this comment.
Pull request overview
This PR reverts NSObject.GetSuper () to an allocation-based approach to reduce the size of NSObjectData, so it can fit within the tagged memory returned by ObjectiveCMarshal.CreateReferenceTrackingHandle (simplifying CoreCLR memory management).
Changes:
- Shrinks the native/managed
NSObjectDatalayout by removing the embeddedobjc_superpointer. - Reintroduces per-
NSObjectnative allocation/caching ofobjc_superused forobjc_msgSendSupercalls. - Updates the dynamic registrar and native runtime struct comments/signatures to match the new
NSObjectDatalayout.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/ObjCRuntime/Registrar.cs | Updates the Objective-C type encoding for xamarinGetNSObjectData to reflect the smaller NSObjectData. |
| src/Foundation/NSObject2.cs | Removes classHandle from NSObjectData and restores an allocation-based GetSuper () implementation (with an optional CWT-backed variant). |
| runtime/xamarin/runtime.h | Updates the native NSObjectData struct definition and associated signature comment to match managed changes. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
* Fix stale Handle in the CWT path: move Handle/ClassHandle assignment outside the GetValue factory so they're updated on every GetSuper call. * Remove stale comment about objc_super being in NSObjectData. * Remove extra blank line after #endif. * Use full URL for issue reference. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Reword the USE_CWT_FOR_SUPER_MEMORY comment to clarify intent. * The FreeData/Dispose pattern matches the old release/8.0.1xx behavior (FreeData only called when handle is zero), so no change needed there. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Re: Dispose/FreeData behavior — I verified against the The copilot reviewer's concern about |
✅ [PR Build #2278dac] Build passed (Detect API changes) ✅Pipeline on Agent |
This comment has been minimized.
This comment has been minimized.
✅ [PR Build #2278dac] Build passed (Build packages) ✅Pipeline on Agent |
✅ API diff for current PR / commitNET (empty diffs)✅ API diff vs stableNET (empty diffs)ℹ️ Generator diffGenerator Diff: vsdrops (html) vsdrops (raw diff) gist (raw diff) - Please review changes) Pipeline on Agent |
✅ [PR Build #2278dac] Build passed (Build macOS tests) ✅Pipeline on Agent |
🔥 [CI Build #2278dac] Test results 🔥Test results❌ Tests failed on VSTS: test results 1 tests crashed, 0 tests failed, 172 tests passed. Failures❌ windows tests🔥 Failed catastrophically on VSTS: test results - windows (no summary found). Html Report (VSDrops) Download Successes✅ cecil: All 1 tests passed. Html Report (VSDrops) Download macOS tests✅ Tests on macOS Monterey (12): All 5 tests passed. Html Report (VSDrops) Download Linux Build VerificationPipeline on Agent |
TODO:
Use the old implementation of NSObject.GetSuper ():
This is less performant, but any performance issues will be alleviated by the fact that NSObject.GetSuper () will be called much less after #25376.
The great advantage is that
NSObjectDatais now smaller, and fits in the tagged memory returned byObjectiveCMarshal.CreateReferenceTrackingHandle, which will simplify memory management a lot for CoreCLR.Contributes towards #25383.