Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Callbacks implementation for MemberCloner #333

Closed
LambdaTheDev opened this issue Jul 16, 2022 · 3 comments
Closed

Callbacks implementation for MemberCloner #333

LambdaTheDev opened this issue Jul 16, 2022 · 3 comments
Labels
dotnet Issues related to AsmResolver.DotNet enhancement
Milestone

Comments

@LambdaTheDev
Copy link

LambdaTheDev commented Jul 16, 2022

Problem Description

When working with MemberCloner, you must do at least 2 loops through all copied types - implicit one (in MemberCloner), and explicit one (when you add types to module).

For small assemblies it won't matter, but for larger - it could cause huge processing time.

Callbacks system can solve this - user can define their callback types, cache processed types (for example interfaces), do some minor processing, and add result automatically to module - in a single iteration.

Proposal

To MemberCloner constructor, add argument Action<IMemberDescriptor>, and invoke it directly after the member is fully cloned.

Alternatives

No response

Additional Context

No response

@Washi1337 Washi1337 added the dotnet Issues related to AsmResolver.DotNet label Jul 17, 2022
@Washi1337 Washi1337 pinned this issue Jul 20, 2022
@Washi1337 Washi1337 unpinned this issue Jul 20, 2022
@Washi1337
Copy link
Owner

I can definitely see value in this.

Perhaps as an alternative to calbacks we could also consider implementing a listener pattern for easier customization and case distinction from the user's perspective. This is particularly useful for users that just want to add their included types to the target module, and thus only need a case for TypeDefinition. Something like the following:

public class MyListener : MemberClonerListener
{
   public override void OnClonedMember(IMetadataMember original, IMetadataMember cloned)
   {
      // called for every member.
   }

   public override void OnClonedType(TypeDefinition original, TypeDefinition cloned)
   {
      // called for every type only
   }

   public override void OnClonedMethod(MethodDefinition original, MethodDefinition cloned)
   {
      // called for every method only.
   }

   // ...
}


var cloner = new MemberCloner(targetModule, new MyListener());

We can easily do both as well. The callback version is just a special case of a full listener version, as can be seen in a potential implementation below:

public class CallbackCloneListener : MemberClonerListener
{
   private readonly Action<IMetadataMember, IMetadataMember> _callback;

   public CallbackCloneListener(Action<IMetadataMember, IMetadataMember> callback)
   {
      _callback = callback;
   }

   public override void OnClonedMember(IMetadataMember original, IMetadataMember cloned) => _callback(original, cloned);
}

var cloner = new MemberCloner(targetModule, (original, cloned) => { ... }); 
// this would be shorthand for;
// var cloner = new MemberCloner(targetModule, new CallbackCloneListener((original, cloned) => { ... }));

@LambdaTheDev
Copy link
Author

Oh, MemberClonerListener idea is even better (split on types, methods, etc)!

@Washi1337
Copy link
Owner

Implemented in #337. This will be part of the 5.0 release.

@Washi1337 Washi1337 added this to the 5.0.0 milestone Aug 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dotnet Issues related to AsmResolver.DotNet enhancement
Projects
None yet
Development

No branches or pull requests

2 participants