Skip to content

Conversation

@vargaz
Copy link
Contributor

@vargaz vargaz commented Oct 24, 2022

No description provided.

@ghost
Copy link

ghost commented Oct 24, 2022

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

Issue Details

null

Author: vargaz
Assignees: vargaz
Labels:

area-AssemblyLoader-mono

Milestone: -

@vargaz vargaz marked this pull request as draft October 24, 2022 19:23
@vargaz vargaz force-pushed the unload2 branch 4 times, most recently from 984a476 to abe02a7 Compare October 25, 2022 04:43
@lateralusX
Copy link
Member

Great start @vargaz! Should we see if we can make some unloading tests work and pass as part of this PR before we merge or should we merge this initial implementation as a wip experimental opt in feature while we work through scenarios case by case?

@vargaz
Copy link
Contributor Author

vargaz commented Oct 25, 2022

Disabled it by default for now.

Copy link
Member

Choose a reason for hiding this comment

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

@vargaz would it be possible to walk stack as part of GC and collect managed frames, use IP's to locate the jit info for the IP's and then from there get the matching types memory manager and LoaderAlloctor object and pin it instead of storing a volatile local variable in each method prolog?

Copy link
Member

Choose a reason for hiding this comment

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

The volatile var hack is kind of nice. you only have to emit it if you're JITing a method from an unloadable ALC, so it's pay for play. I don't know if we plan to support AOT with unloadable ALCs, in which case I guess we need some other idea, but for the JIT and interp it's kind of a nice hack

Copy link
Member

Choose a reason for hiding this comment

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

Anyone aware of anything fundamentally blocking us to support AOT with unloadable ALC's?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The AOT code is associated with a specific loaded assembly, so it can't handle the case where an assembly is loaded multiple times in multiple ALCs, or unloaded.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vargaz would it be possible to walk stack as part of GC and collect managed frames, use IP's to locate the jit info for the IP's and then from there get the matching types memory manager and LoaderAlloctor object and pin it instead of storing a volatile local variable in each method prolog?

It is possible but it having to do a stack walk for all threads would slow down GCs a lot.

Copy link
Member

Choose a reason for hiding this comment

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

OK, yes, with a lot of threads that would have impact on GC and maybe an additional volatile store to the stack per method prolog will be fast enough and as long as we do conservative stack scanning of complete thread stack it will automatically be kept alive.

Copy link
Member

@lateralusX lateralusX Oct 26, 2022

Choose a reason for hiding this comment

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

The AOT code is associated with a specific loaded assembly, so it can't handle the case where an assembly is loaded multiple times in multiple ALCs, or unloaded.

I suspected that was the case, they have similar limitation in R2R, so not unique to Mono, https://github.com/dotnet/runtime/blob/main/docs/design/features/assemblyloadcontext.md#constraints.

Will that case be handled in some way, if you try to load the same AOT:ed assembly in different ALC, will the second load then JIT the methods and if running full AOT, fail and assert or will it share the same native method implementation in both ALC's?

I guess the best way to handle AOT:ed assemblies is to load them into the default ALC and then use JIT or Interpreter for code loaded in custom ALC's.

Comment on lines +42 to +44
Copy link
Member

Choose a reason for hiding this comment

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

I'm doing a very similar thing when hot reload adds instance fields to an existing object.

https://github.com/dotnet/runtime/pull/76462/files#diff-b2557f22ad994774fd5dc5cb8b555e4cf5dbfcc07b81f05e44866ca7e6442bc5

Maybe there's something common we can share?

Copy link
Member

Choose a reason for hiding this comment

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

The volatile var hack is kind of nice. you only have to emit it if you're JITing a method from an unloadable ALC, so it's pay for play. I don't know if we plan to support AOT with unloadable ALCs, in which case I guess we need some other idea, but for the JIT and interp it's kind of a nice hack

@build-analysis build-analysis bot mentioned this pull request Oct 25, 2022
2 tasks
@vargaz vargaz marked this pull request as ready for review October 25, 2022 20:16
@vargaz vargaz force-pushed the unload2 branch 3 times, most recently from ed7d88c to f507003 Compare October 26, 2022 08:56
… called with a non-zero context and a non-generic signature.
on the GC heap.

Normally, static variables are stored in an array inside MonoVTable
which is registered as a GC root. For collectible alcs, this would
not work, since GC references in these arrays would keep the alc alive.
Instead, store them in arrays referenced by the LoaderAllocator object.
This assumes the static variables will no longer be accessed after
the LoaderAllocator object dies.
Copy link
Member

@lateralusX lateralusX left a comment

Choose a reason for hiding this comment

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

Great start! LGTM!

@vargaz vargaz force-pushed the unload2 branch 3 times, most recently from af2acc4 to cb3076f Compare December 13, 2022 15:54
@vargaz
Copy link
Contributor Author

vargaz commented Dec 13, 2022

/azp run runtime

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@vargaz
Copy link
Contributor Author

vargaz commented Dec 15, 2022

The interpreter failures are relevant.

@vargaz
Copy link
Contributor Author

vargaz commented Dec 15, 2022

Failures are unrelated.

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.

3 participants