Skip to content

Commit

Permalink
[mono] Hot Reload: initial push to support adding lambdas (#63513)
Browse files Browse the repository at this point in the history
Adds support for:
* Adding static lambdas to existing methods, whether or not they had lambdas before.
* Adding lambdas that capture `this` to existing methods that already captured `this` before
* Adding new static or instance methods to existing types
* Adding static fields to existing types
* Adding new types (note that adding instance fields currently will fail.  It'll go in in a future PR that will address #63643).

Note that reflection on newly added members isn't tested yet and likely doesn't work.

Fixes #50249

Contributes to #57365

* Add AddStaticField test; cleanup AddLambdaCapturingThis

* Add AddNestedClass testcase

* hot_reload: add a comment about remaining work

* move per-class metadata update info to MonoClassMetadataUpdateInfo

   Use a property on the class instead of a hash table in the baseline image to keep track of added fields and methods

* Cleanup pass1 to check add/update against prior gen

   Instead of the baseline image

* adding a class

   Working on making mono_metadata_nested_in_typedef do the right thing for nested classes

   Contributes to making mono_class_create_from_typedef do the right thing for nested classes.

* fix: correct mutant table population when the baseline table is empty

   In that case the table row_size and size_bitmap of the baseline are both 0, so offset calculations don't work.

   In that case when we know we need to add rows to the mutant table, use the uncompressed layout (32-bit columns, always) same as the EnC delta.

* Calling new methods on a nested class works

* adding fields

   Doesn't work yet.  Dies in mono_class_get_field, called from mono_field_from_token_checked.

   The problem we're going to have to deal with is that unlike MonoMethod which is allocated upon lookup, the MonoClassFields of a class are allocated contiguously in class initialization.  And there are not really enough fields in MonoClassField to indicate "this is not an ordinary field, don't do pointer arithmetic with it".  One idea is to steal a bit from MonoClassField:parent and always check it as a trigger to go on a slower path.

* adding static fields

   Rework the delta info to store info about "members" (rather than methods and fields separately).

   Add a MonoClassMetadataUpdateField subclass of MonoClassField that has extra info.

   Add new component methods to go from a MonoClassField to an index, and from a field token to a MonoClassField.

   Set MonoClassField:offset to -1 to generally go down the "special static field" codepath since we will need to do some one-off work to make a storage area for added static fields.

   Instance fields are not supported yet and error out during the update pass for now.

* fix custom attribute lookups on new classes/methods/fields

* Add a sketch of HotReloadInstanceFieldTable

   This is a class that manages the lifetime for added instance fields for reference types that had new fields added to them by EnC   It's basically ConditionalWeakTable<object, Dictionary<uint, Store>> where the outer key is the object instance, the inner key is the fielddef token, and the value is a Store.  The store has a single object? field which the location where the field's value will go.  For reference types this is the reference directly, and for valuetypes and primitives, this is the boxed object (so to get the actual storage address you need to add sizeof(MonoObject)).

* mono_metadata_update_get_static_field_addr

   two reminders:

   1. The static field area needs to be GC-visible
   2. We should really handle added fields on un-inited classes by allocating the instances in the normal way - in the layout algorithm.

* Free MonoClassMetadataUpdateInfo when BaselineInfo is destroyed

* placeholder struct for runtime class data; instance offset placeholder

* static field storage

   Need to store the fields in a GC root, and in pinned allocations.

* Implement hot_reload_get_static_field_addr()

* Add mono_metadata_update_find_method_by_name

   Now we can find .cctor

   This is enough that a basic sample with an added static lambda works

* Fix infinite loop

* fix build

* fix dynamic components builds

* fix windows build

* Fix inadvertent fallthru in previous fix

* Report new capabilities

   NewTypeDefinition is not completely functional because adding instance fields is not supported yet.   But adding classes with only static methods works.

* tests: describe what existing tests do, add placeholder static lambda test

* tests: Add AddStaticLambda test case

   Adding a brand new static lambda to an existing method body is supported

* Destroy the runtime part of MonoClassMetadataUpdateInfo, too

* rename Mono.HotReload file

   it has more classes than just the instance table

* tests: add ActiveIssue for supporting adding instance fields

* ifdef out Mono.HotReload.InstanceFieldTable

   it's not ready yet

* Remove get_added_members from hot reload component interface

   Doesn't seem necessary to expose this yet.  It's only used in hot_reload.c right now

* Change the AddStaticLambda sample to use Func<string,string>

   To check that lambdas with parameters work

* Use a mempool allocated GSlist for the added members

   This avoids invalidating iterators in case we add members on one thread while another thread is iterating.

   If it turns out we need random access, we can switch to a GArray with locking.  But so far we only ever iterate.

* Use mono_get_corlib instead of passing MonoDefaults to hot_reload

* use normal GENERATE_TRY_GET_CLASS_WITH_CACHE

* [metadata] make m_field_set_parent and m_field_set_meta_flags inline

   m_ prefix functions are supposed to be inline
  • Loading branch information
lambdageek authored Jan 18, 2022
1 parent d2844eb commit c50185b
Show file tree
Hide file tree
Showing 38 changed files with 1,366 additions and 194 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,21 @@ namespace System.Reflection.Metadata.ApplyUpdate.Test
{
public class AddLambdaCapturingThis
{
public AddLambdaCapturingThis () {
field = "abcd";
}
public AddLambdaCapturingThis()
{
field = "abcd";
}

public string GetField => field;
public string GetField => field;

private string field;
private string field;

public string TestMethod () {
// capture 'this' but no locals
Func<string,string> fn = s => field;
return "123";
}
public string TestMethod()
{
// capture 'this' but no locals
Func<string,string> fn = s => field;
return "123";
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,21 @@ namespace System.Reflection.Metadata.ApplyUpdate.Test
{
public class AddLambdaCapturingThis
{
public AddLambdaCapturingThis () {
field = "abcd";
}
public AddLambdaCapturingThis()
{
field = "abcd";
}

public string GetField => field;
public string GetField => field;

private string field;

public string TestMethod () {
// capture 'this' but no locals
Func<string,string> fn = s => NewMethod (s + field, 42);
return fn ("123");
}

private string NewMethod (string s, int i) {
return i.ToString() + s;
}
private string field;

public string TestMethod()
{
// capture 'this' but no locals
Func<string,string> fn = s => field;
Func<string,string> fn2 = s => "42" + s + field;
return fn2 ("123");
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
using System;


namespace System.Reflection.Metadata.ApplyUpdate.Test
{
public class AddNestedClass
{
public AddNestedClass()
{
}

public string TestMethod()
{
return "123";
}

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
using System;


namespace System.Reflection.Metadata.ApplyUpdate.Test
{
public class AddNestedClass
{
public AddNestedClass()
{
}

public string TestMethod()
{
var n = new Nested();
n.f = "123";
return n.M();
}

private class Nested {
public Nested() { }
internal string f;
public string M () {
return f + "456";
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<RootNamespace>System.Runtime.Loader.Tests</RootNamespace>
<TargetFrameworks>$(NetCoreAppCurrent)</TargetFrameworks>
<TestRuntime>true</TestRuntime>
<DeltaScript>deltascript.json</DeltaScript>
</PropertyGroup>
<ItemGroup>
<Compile Include="AddNestedClass.cs" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"changes": [
{"document": "AddNestedClass.cs", "update": "AddNestedClass_v1.cs"},
]
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
using System;


namespace System.Reflection.Metadata.ApplyUpdate.Test
{
public class AddStaticField
{
public AddStaticField () {
}

public string GetField => s_field;

private static string s_field;

public void TestMethod () {
s_field = "abcd";
}

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
using System;


namespace System.Reflection.Metadata.ApplyUpdate.Test
{
public class AddStaticField
{
public AddStaticField () {
}

public string GetField => s_field2;

private static string s_field;

private static string s_field2;

public void TestMethod () {
s_field = "spqr";
s_field2 = "4567";
}

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<RootNamespace>System.Runtime.Loader.Tests</RootNamespace>
<TargetFrameworks>$(NetCoreAppCurrent)</TargetFrameworks>
<TestRuntime>true</TestRuntime>
<DeltaScript>deltascript.json</DeltaScript>
</PropertyGroup>
<ItemGroup>
<Compile Include="AddStaticField.cs" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"changes": [
{"document": "AddStaticField.cs", "update": "AddStaticField_v1.cs"},
]
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
using System;


namespace System.Reflection.Metadata.ApplyUpdate.Test
{
public class AddStaticLambda
{
public string TestMethod()
{
return "abcd";
}

public string Double(Func<string,string> f) => f("") + f("1");

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
using System;


namespace System.Reflection.Metadata.ApplyUpdate.Test
{
public class AddStaticLambda
{
public string TestMethod()
{
return Double(static (s) => s + "abcd");
}

public string Double(Func<string,string> f) => f("") + f("1");

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<RootNamespace>System.Runtime.Loader.Tests</RootNamespace>
<TargetFrameworks>$(NetCoreAppCurrent)</TargetFrameworks>
<TestRuntime>true</TestRuntime>
<DeltaScript>deltascript.json</DeltaScript>
</PropertyGroup>
<ItemGroup>
<Compile Include="AddStaticLambda.cs" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"changes": [
{"document": "AddStaticLambda.cs", "update": "AddStaticLambda_v1.cs"},
]
}

96 changes: 82 additions & 14 deletions src/libraries/System.Runtime.Loader/tests/ApplyUpdateTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ void LambdaBodyChange()
[ActiveIssue("https://github.com/dotnet/runtime/issues/54617", typeof(PlatformDetection), nameof(PlatformDetection.IsBrowser), nameof(PlatformDetection.IsMonoAOT))]
void LambdaCapturesThis()
{
// Tests that changes to the body of a lambda that captures 'this' is supported.
ApplyUpdateUtil.TestCase(static () =>
{
var assm = typeof (ApplyUpdate.Test.LambdaCapturesThis).Assembly;
Expand Down Expand Up @@ -263,25 +264,92 @@ public void AsyncMethodChanges()
});
}

[ActiveIssue ("https://github.com/dotnet/runtime/issues/50249", TestRuntimes.Mono)]
[ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))]
public static void TestAddLambdaCapturingThis()
{
ApplyUpdateUtil.TestCase(static () =>
{
var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis).Assembly;
[ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))]
public static void TestAddLambdaCapturingThis()
{
// Test that adding a lambda that captures 'this' (to a method that already has a lambda that captures 'this') is supported
ApplyUpdateUtil.TestCase(static () =>
{
var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis).Assembly;
var x = new System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis();
Assert.Equal("123", x.TestMethod());
ApplyUpdateUtil.ApplyUpdate(assm);
string result = x.TestMethod();
Assert.Equal("42123abcd", result);
});
}

[ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))]
public static void TestAddStaticField()
{
// Test that adding a new static field to an existing class is supported
ApplyUpdateUtil.TestCase(static () =>
{
var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField).Assembly;
var x = new System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField();
x.TestMethod();
Assert.Equal ("abcd", x.GetField);
ApplyUpdateUtil.ApplyUpdate(assm);
x.TestMethod();
string result = x.GetField;
Assert.Equal("4567", result);
});
}

[ActiveIssue("https://github.com/dotnet/runtime/issues/63643", TestRuntimes.Mono)]
[ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))]
public static void TestAddNestedClass()
{
// Test that adding a new nested class to an existing class is supported
ApplyUpdateUtil.TestCase(static () =>
{
var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass).Assembly;
var x = new System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass();
var r = x.TestMethod();
Assert.Equal ("123", r);
ApplyUpdateUtil.ApplyUpdate(assm);
r = x.TestMethod();
var x = new System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis();
Assert.Equal("123456", r);
});
}

[ConditionalFact(typeof(ApplyUpdateUtil), nameof(ApplyUpdateUtil.IsSupported))]
public static void TestAddStaticLambda()
{
// Test that adding a new static lambda to an existing method body is supported
ApplyUpdateUtil.TestCase(static () =>
{
var assm = typeof(System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda).Assembly;
Assert.Equal("123", x.TestMethod());
var x = new System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda();
ApplyUpdateUtil.ApplyUpdate(assm);
var r = x.TestMethod();
string result = x.TestMethod();
Assert.Equal("42123abcd", result);
});
}
Assert.Equal ("abcd", r);
ApplyUpdateUtil.ApplyUpdate(assm);
r = x.TestMethod();
Assert.Equal("abcd1abcd", r);
});
}

class NonRuntimeAssembly : Assembly
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@
<ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.LambdaCapturesThis\System.Reflection.Metadata.ApplyUpdate.Test.LambdaCapturesThis.csproj" />
<ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate\System.Reflection.Metadata.ApplyUpdate.Test.FirstCallAfterUpdate.csproj" />
<ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis\System.Reflection.Metadata.ApplyUpdate.Test.AddLambdaCapturingThis.csproj" />
<ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField\System.Reflection.Metadata.ApplyUpdate.Test.AddStaticField.csproj" />
<ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass\System.Reflection.Metadata.ApplyUpdate.Test.AddNestedClass.csproj" />
<ProjectReference Include="ApplyUpdate\System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda\System.Reflection.Metadata.ApplyUpdate.Test.AddStaticLambda.csproj" />
</ItemGroup>
<ItemGroup Condition="'$(TargetOS)' == 'Browser'">
<WasmFilesToIncludeFromPublishDir Include="$(AssemblyName).dll" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,7 @@
<Compile Include="$(LibrariesProjectRoot)\System.Private.CoreLib\src\System\Threading\ThreadPoolBoundHandle.PlatformNotSupported.cs" />
</ItemGroup>
<ItemGroup>
<Compile Include="$(BclSourcesRoot)\Mono\HotReload.cs" />
<Compile Include="$(BclSourcesRoot)\Mono\RuntimeStructs.cs" />
<Compile Include="$(BclSourcesRoot)\Mono\RuntimeMarshal.cs" />
<Compile Include="$(BclSourcesRoot)\Mono\SafeStringMarshal.cs" />
Expand Down
Loading

0 comments on commit c50185b

Please sign in to comment.