Skip to content

Conversation

@CoffeeFlux
Copy link
Contributor

Various functionality is behind build defines due to both stability and performance concerns.

Remaining work for this PR:

  • Scan RefTracker from objects
  • Scan RefTracker from stack walk
  • Get inflated method properly in mono_memory_manager_from_method
  • Rebase

Remaining long-term work:

  • Add and populate keepalive field with RefTracker for reflection types
  • Update remaining code memory allocations (mostly JIT and AOT)
  • Put MemoryManager/RefTracker pointers directly in vtables
  • Update docs
  • Add MONO_ROOT_SOURCE_MEMORY_MANAGER (or something like that)
  • Add exclusions for creating certain types within collectible ALCs
  • Turn on by default for netcore

@ghost
Copy link

ghost commented Jul 14, 2020

Tagging subscribers to this area: @CoffeeFlux
Notify danmosemsft if you want to be subscribed.

Copy link
Member

@lambdageek lambdageek left a comment

Choose a reason for hiding this comment

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

I'd err on the side of being paranoid about locking, for now, during unloading.

Overall this seems like it will work, but I think for generic instance objects we'll need a GenericRefTracker managed object that references the set of ReferenceTrackers the instance could come from.

Copy link
Member

Choose a reason for hiding this comment

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

You will need to pass some kind of user_data to handle_ref_trackers_from_frame so that it call call (GcScanFunc)ctx.ops->copy_or_mark_object (see how mono_handle_stack_scan does it, below) during the precise pass.

Copy link
Member

Choose a reason for hiding this comment

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

It's supposed to scan the obj->vtable->klass->image->alc->ref_tracker gc handle for a non-generic object and
for a generic instance it's each_ref_tracker_of_image_set (((MonoClassGenericInst*)obj->vtable->klass)->generic_class->owner) something like that?

So if we put something in the vtable directly, for a generic instance we'd need some kind of GenericRefTracker that points at an array of regular ref trackers, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

What does the _code suffix mean here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just that it's code memory, though maybe that's unnecessary. I wasn't sure on the nomenclature for a bunch of this.

@vargaz
Copy link
Contributor

vargaz commented Jul 28, 2020

Would be nice to split parts of this into smaller PRs, its huge.

@CoffeeFlux
Copy link
Contributor Author

Yeah, I'll push the next round of changes and then start chunking it up into separate PRs. Some of the commits should already be reasonably-sized at least.

@srxqds
Copy link
Contributor

srxqds commented Aug 6, 2020

excited work,why not this pr merged first?

@CoffeeFlux
Copy link
Contributor Author

I'm currently on vacation, so I wouldn't expect this PR to move a whole lot right now :)

@srxqds
Copy link
Contributor

srxqds commented Aug 14, 2020

when can this pr be merged?
and when continue to begin the remain work?
I think I should make it myself because our project can not waiting for the complete work.
if this pr merge what job should we do to make alc supported unloading assembly?

@srxqds
Copy link
Contributor

srxqds commented Aug 20, 2020

?

@CoffeeFlux
Copy link
Contributor Author

I'll be working on this more next week. It will not be part of the .NET 5 release; that has already branched without this PR, and master is now .NET 6.

@srxqds
Copy link
Contributor

srxqds commented Aug 21, 2020

OK, thank for your reply, if this feature work, we will begin with .net6.
is there release date of .net6 preview1?

@CoffeeFlux CoffeeFlux closed this Sep 9, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants