jimingham wrote: > > This seems like a generic module loading observer. I don't see anything JIT > > specific about it. Not saying a generic module loading observer is not a > > good idea. But calling it a JITLoader seems pretty confusing to me. > > true, in the current form. Check out the comments in my previous reply about > the motivations for this. > > > One thing that's nicer about your approach here over independent callbacks > > for each of the hooks is that it allows you to group the three callbacks in > > the same class instance, so that you can more easily keep shared state. > > But that's a general problem with these affordances. For instance, it's > > super inconvenient that the summary providers and synthetic child providers > > produce separate objects to handle the callbacks. You end up computing the > > sizes of container classes twice, once for the summary and once because you > > need it to present the type... If the summary & child provider could share > > state, you'd only need to do that once per stop. > > And as we are adding more of these callbacks for "lifecycle events" it > > would be really convenient, as you have done here, to be able to get one > > object to manage multiple different types of callback hits. > > For the case of hooks, I wonder if we could augment the -C options we > > currently use to add affordances backed by a Python class with a `__call__` > > method so you could say: > > `target stop-hook add -C my_python_class --shared-instance 1 --call_method > > stop_hook_handler ` > > and then to finish off your design, we'd add: > > `target module-hook add -C my_python_class --shared-instance 1 > > --call_method module_hook_handler ` > > The --shared-instance that would tell lldb to make a single object instance > > and reuse if for any --shared-instance addition that uses this class (one > > for each target in this case). Allowing you to specify the method name > > means you don't have to do the discrimination in `__call__`... > > That way as we add more of these callbacks (which I agree we really need to > > do) people could mix and match them as the wish, and we wouldn't have to > > figure out the right combination(s) for everybody's use cases. > > Note for summary & synthetic child providers you wouldn't need to specify > > the methods as those are pre-determined. So the `--call-method` would not > > be necessary. In their case the shared instance would be held by a > > ValueObject, since we make formatter instances for each value object we > > format. > > It might also be handy to be able to define several commands that share > > state, so you could do the same thing for command classes (though in that > > case the shared instance would be held by the debugger not the target...) > > It would be great to create a generic version of this that allows users to > add methods to a python class and just have it be notified. That being said, > by motivation here is to augment the JITLoader plug-in API to do more things > as mentioned above. Now we can probably do this with a notification class > that we can install as long as we can find out about everything that is > needed. So one approach might be to allow a python class to get all of the > notifications I need for the JITLoader plug-in without calling it a JITLoader > as you wanted. The class could be something like: > > ``` > class TargetObserver: > def __init__(self, target): > # Created as soon as a target is created > self.target = target > > # Module observing > def module_added(self, module): > # Called anytime a module is added to the target > pass > def module_removed(self, module): > # Called anytime a module is removed from the target > pass > def module_loaded(self, module): > # Called when the module is loaded or modified by the dynamic loader > pass > > # Launch/Attach observing > def did_launch(self): > # Called when the target is done being launched > pass > def did_attach(self): > # Called when the target is done being attached > pass > > # Breakpoint observing > def breakpoint_added(self, breakpoint): > # Called when a breakpoint is added to the target > pass > def breakpoint_modified(self, breakpoint): > # Called when a breakpoint is modified to the target > pass > def breakpoint_deleted(self, breakpoint): > # Called when a breakpoint is added to the target > pass > > # Unresolved address resolving observation > def resolve_load_address(self, load_addr): > # Allow plug-ins to help resolve addresses that don't resolve in the > current target > pass > ``` > > We could remove the JITLoader class all together.
I think I'd come at if from the opposite direction. We don't currently know what the full set of messages that we want to send are, so making one class that receives all the messages we know about at present seems limiting. What I was proposing instead is that when we add a way to register a callback to some event in lldb, we extend the registry to indicate not just the class that will be instantiated to watch the event, but which method is the responder. That way, for instance, you can register a stop-hook with your class, and then you will have launch and attach callbacks already. But you need a way to say "Use a common instance of this class per-whatever entity owns that callback" and "use this method(s) on my object". That was we don't have to hook everything up for you, but rather it will be easy for designers to make a class where they can hook up the particular callbacks they need. https://github.com/llvm/llvm-project/pull/142514 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
