Open mmurrell opened 3 years ago
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.
This would go against the W^X work that's already been done which is a big deal going forward.
In the API Usage example above, wouldn't all the code be compiled in advance, and not limited by W^X? Both the original implementation and the replaced delegate would be loaded in 'execute' pages. Unless I misunderstand, switching the method by reflection would be more akin to changing an attribute on the object level, akin PropertyInfo.SetValue()
and not blocked by W^X.
Perhaps my comparisons with Reflection.Emit
break down in light of W^X, but I would hope the original argument still stands.
Non virtual methods are bound to type, not instance. Virtual method table is stored in type information, not directly in object. The CLR code structure does not allow modifying method body by instance. Changing so would likely to break performance of "common" code.
Pathed this to VM because I'm not sure where else it can go. At minimum it would affect the type system and JIT, possibly also p/invoke and calli stub invocation.
Your are mixing OOP concepts of methods (per class) and fields/properties (per instance). If something really differs at the instance granularity, C# has delegates and you are free to assign different delegates as a field or property to your checker instances.
So when will this feature be on the agenda? I don't care if you replace the assembly or some other way.
Your are mixing OOP concepts of methods (per class) and fields/properties (per instance). If something really differs at the instance granularity, C# has delegates and you are free to assign different delegates as a field or property to your checker instances.
你说了句废话! 懒得翻译英文回你.
I like this API! And~ Can we add some way to close and open this feature, like the: add the env or app config or compile options?
Your are mixing OOP concepts of methods (per class) and fields/properties (per instance). If something really differs at the instance granularity, C# has delegates and you are free to assign different delegates as a field or property to your checker instances.
你说了句废话! 懒得翻译英文回你.
This is not your issue. If you don't want to reply, then don't reply.
Also please obey Code of Conduct.
@NMSAzulX in this repo there are contributors from around the world but we use English as a common language. Also, please take care to follow the code of conduct.
Your are mixing OOP concepts of methods (per class) and fields/properties (per instance). If something really differs at the instance granularity, C# has delegates and you are free to assign different delegates as a field or property to your checker instances.
你说了句废话! 懒得翻译英文回你.
This is not your issue. If you don't want to reply, then don't reply.
Also please obey Code of Conduct.
Dear fellow Canadians, you made a meaningless remark. I don't think you understand his scene at all. This scene is very important.
@acaly I don't know if you are using .NET as the programming language, but Delegate as fields and properties is basic knowledge, as we all know, you respond to the features expected by others with basic knowledge, such as XCYB. Am I right? Now you should know that the function he expects is more different.
OK. Then what is the problem of using delegates as properties in his scenario?
Note that his original code replaces methods for a single instance. If that is to be allowed, we should expect user to replace methods for many different instances. Obviously, if you have basic knowledge, "replace the assembly" as you said won't be a solution here even it somehow works, because it would potentially gives you many copies of the assembly, which is unacceptable considering the performance impact, memory footprint and safety.
One may expect a direct method call to be faster than calling a delegate. That is true, but the difference is sufficiently small. That is the reason why delegates are everywhere in .NET. If the purpose was to replace a method that can be called in a faster way than using delegates, I don't think it should ever be made to the runtime. Besides this consideration, I don't know other reason for not using delegates in his original scenario.
There are other scenarios such as hot replace, but it is not clear if the original question was to ask for such functions.
Also I am an international student currently living in Canada. My nationality is Chinese. This is not important to this discussion but it seems you care a lot about it.
@acaly Profiling api can do that, so why can't you support it in runtime? Canada is a beautiful country and their air is sweet. I just think you answered in a hurry.
Which profiling API can replace methods per instance? And again, my question was, what is the problem of using delegates as properties in his scenario?
If you don't explain clearly what you want to achieve, why existing methods don't help you, and how it can be made possible, then you don't expect the proposal to go anywhere.
Which profiling API can replace methods per instance? And again, my question was, what is the problem of using delegates as properties in his scenario?
If you don't explain clearly what you want to achieve, why existing methods don't help you, and how it can be made possible, then you don't expect the proposal to go anywhere.
The profiling api must be in C++ and is rather complex,officially does not recommend this either。 https://docs.microsoft.com/zh-cn/dotnet/framework/unmanaged-api/profiling/profiling-overview#unsupported-functionality https://docs.microsoft.com/zh-cn/dotnet/framework/unmanaged-api/profiling/icorprofilerinfo-getilfunctionbody-method https://docs.microsoft.com/zh-cn/dotnet/framework/unmanaged-api/profiling/icorprofilerinfo-setilfunctionbody-method https://docs.microsoft.com/zh-cn/dotnet/framework/unmanaged-api/profiling/icorprofilerinfo4-requestrejit-method
@acaly Why do you require him to use fields and properties? We have to admit that Delegate can solve the problem of method replacement. If so, is it necessary for the method to exist? If everyone uses Delegate, there won't be this issue, and of course we can't let everyone do that. His focus is on this API. And this API will bring more benefits, such as AOP, such as upgrading old programs that lose source code, such as updating services that cannot be stopped, and so on.
Which profiling API can replace methods per instance? And again, my question was, what is the problem of using delegates as properties in his scenario?
Note that I said per instance. Those are just replacing single method. My first comment was also about replacing for single instance. If it was about replacing for all instances, then I wouldn't suggest using delegates.
If so, is it necessary for the method to exist? If everyone uses Delegate, there won't be this issue
I hope you know lambda and delegate are themselves implemented with methods.
We have to admit that Delegate can solve the problem of method replacement.
You mean that this proposal is not necessary, don't you?
And this API will bring more benefits, such as AOP, such as upgrading old programs that lose source code, such as updating services that cannot be stopped, and so on.
For what you've suggested, replacing the behavior of an instance from external code is completely against OOP. It will make everyone who maintains public APIs to worry about how user will use their library.
@acaly Don't try to negate his API. If the method can be replaced, then he needs to control whether the previous method is called or the later method is called. The premise is that we can replace the method. As for the safety factor, I think this is the most useless focus. Because there are many technologies that can destroy it, certificate verification or other verification mechanisms can be added to the replacement function.
typeof(Doer).AddVerity();
var method = typeof(Doer).GetMethod("DoA");
var handler = method.Vertify(....);
handler.Replace(.....);
Maybe there has some real scence to use this feature. If you are the programer who write the client code, use the xLua you can do this (the code below copy from the internet):
print("*********multiple method replace***********")
xlua.hotfix(CS.HotfixMain, {
Update = function(self)
print(os.time())
end,
Add = function(self, a, b )
return a + b
end,
Speak = function(a)
print(a)
end
})
xlua.hotfix(CS.HotfixTest, {
[".ctor"] = function()
print("Lua hot fix ctor")
end,
Speak = function(self,a)
print("UnitySir" .. a)
end,
Finalize = function()
end
})
And this feature always uses to hotfix some little bug or dynamic change the client behavior.
The DotNet has this feature, but it is very hard to use. So if we have this API, we can do this easily. So, what reason makes us negate this API?
While maybe this API getting the application's risks, but I think this feature needs the switch to close. So why we don't like It?
Ho forgot the little thing, most of the U3D's development use the C# and xLua to realization this feature.
Replacing non-virtual method per instance is technically impossible. The method is often inlined before getting the object. Disable inlining or even add a check will downgrade performance a lot.
Don't try to negate his API.
I cannot negate this API. I am just commenting my own thinking on this API. Every API will go through official API review.
then he needs to control whether the previous method is called or the later method is called
certificate verification or other verification mechanisms can be added to the replacement function
How? Those need to be added to the API proposal if they are necessary for it to work.
It's not possible to replace a non-virtual method body at runtime (some methods even get inlined after JIT and no actual code generated for them dedicately, you can reassign delegates because they're actually function pointers), you can manipulate il as a build task after compiled as you want.
For example, using Fody: https://github.com/Fody/Fody
Maybe there has some real scence to use this feature. If you are the programer who write the client code, use the xLua you can do this (the code below copy from the internet):
print("*********multiple method replace***********") xlua.hotfix(CS.HotfixMain, { Update = function(self) print(os.time()) end, Add = function(self, a, b ) return a + b end, Speak = function(a) print(a) end }) xlua.hotfix(CS.HotfixTest, { [".ctor"] = function() print("Lua hot fix ctor") end, Speak = function(self,a) print("UnitySir" .. a) end, Finalize = function() end })
And this feature always uses to hotfix some little bug or dynamic change the client behavior.
The DotNet has this feature, but it is very hard to use. So if we have this API, we can do this easily. So, what reason makes us negate this API?
While maybe this API getting the application's risks, but I think this feature needs the switch to close. So why we don't like It?
Ho forgot the little thing, most of the U3D's development use the C# and xLua to realization this feature.
java has this feature. very easy to use For example, java asm, javaassit, java instrumentation
Addressing some of the comments on the thread:
Regarding delegates -I understand their use; I just don't think it applies here. My very specific API request was to support better isolation for testing, and that can already be done by declaring an interface for every class, no matter how small. However, the cost (development, maintainability, complexity) of maintaining all those extra declarations is non-trivial. The same problem would apply with delegates. -- As a contrast, the API request is a way to perform a common task, minimizing the amount of overhead or friction to developers.
Which profiling API can replace methods per instance? And again, my question was, what is the problem of using delegates as properties in his scenario?
The profiling API was the 'proprietary techniques' I cited in the original post referencing the commercial products. Yes, the profiling API does allow method interception, albeit on a class level (not instance), but I agree with the other commenter that it is prohibitively complex / cumbersome to implement. That is a very large amount of risk, complexity, and overhead to consider for common use. I would consider method interception as an alternative to this proposal, if such a feature were more easily available in the runtime.
virtual method per instance is technically impossible. The method is often inlined before getting the object. Disable inlining or even add a check will downgrade performance a lot.
For what you've suggested, replacing the behavior of an instance from external code is completely against OOP. It will make everyone who maintains public APIs to worry about how user will use their library.
I would hope that allowing developers to opt-in to replacement could mitigate these concerns.
On the general topic of class vs instance replacement, if we're opening up the source to add this feature, instance based method replacement would be ideal. If that is somehow prohibitive, class based method replacement would suffice.
And finally, though I'm not sure how this is relevant, I'm a big fan of Canada. =)
If you are mainly worried about creating too many interface implementations, would inline anonymous types help? There are proposals in C# to allow user to implement interfaces with anomymous types inside method body like in Java.
Background and motivation
It would be nice if the framework supported a way to dynamically replace a method body of an instance of an object,at runtime. I can think of several cases where a pattern like this could be leveraged, but the primary use case for me would support test automation.
As an example, say I'd like to test a class that has multiple dependencies:
The predominant seam for isolating the SUT from the dependencies would be to litter interfaces throughout the code (
IChecker
andIDoer
). This is fine in the naive case, but in larger projects this causes a proliferation of single-use interfaces. If the framework supported replacing method bodies, one could simply create a newChecker
object and replaceCheck()
withreturn true
orreturn false
, and replaceDoA()
with an assertion to pass or fail.This concept is not novel and already exists in other frameworks (eg Ruby). Also, proving the demand for such an extension, there are commercial offerings which use proprietary techniques to accomplish a similar goal, but these are expensive, complex, and fragile. I believe these attach to the CLR Profiler to intercept method invocation, but I'm probably wrong...
API Proposal
I would like to propse the API should support a reflection option where a method body could be replaced at the instance level of an object with a delegate of matching signature:
API Usage
Extending the testing example above, the API could be used as follows.
Risks
While there are risks of application instability, I'd suggest those are owned by the developer. Although my perspective is limited, I believe this would offer no greater risk than a developer writing poor code using Reflection.Emit.
The larger risk, in my mind is the security risk associated with letting users replace random methods. There are a few options here for mitigation, but I would hope that this could be mitigated by an attribute allowing a user to opt-in or opt-out of having their methods replaced. Microsoft may choose to opt-out all System assemblies by default, opting-in certain assemblies or methods they deem valuable, and consumers could opt-in their assemblies or classes at their own discretion.
Thanks for your consideration!