scijava / scijava-common

A plugin framework and application container with built-in extensibility mechanism :electric_plug:
BSD 2-Clause "Simplified" License
85 stars 53 forks source link

Commands get instantiated multiple times per run #476

Open ctrueden opened 3 months ago

ctrueden commented 3 months ago

As discussed in this Zulip thread, when invoking a SciJava Command plugin, multiple instances of the command might get instantiated and then thrown away, just to retrieve default parameter values. This can create confusion if there is actual business logic that gets executed during object construction, because it will then be executed multiple times, and if there are side effects, and/or performance implications, they will occur repeatedly.

Stack trace illustrating the multiple instantiations ``` Breakpoint reached at sc.iview.commands.add.AddBox.(AddBox.kt:75) Breakpoint reached at sc.iview.commands.add.AddBox.(AddBox.kt:75) at java.lang.invoke.DirectMethodHandle$Holder.newInvokeSpecial(DirectMethodHandle$Holder:-1) at java.lang.invoke.Invokers$Holder.invokeExact_MT(Invokers$Holder:-1) at jdk.internal.reflect.DirectConstructorHandleAccessor.invokeImpl(DirectConstructorHandleAccessor.java:86) at jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62) at java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:502) at java.lang.reflect.ReflectAccess.newInstance(ReflectAccess.java:128) at jdk.internal.reflect.ReflectionFactory.newInstance(ReflectionFactory.java:304) at java.lang.Class.newInstance(Class.java:725) at org.scijava.plugin.PluginInfo.createInstance(PluginInfo.java:304) at org.scijava.command.CommandInfo.createInstance(CommandInfo.java:248) at org.scijava.command.CommandModule.instantiateCommand(CommandModule.java:248) at org.scijava.command.CommandModule.(CommandModule.java:98) at org.scijava.command.CommandInfo.createModule(CommandInfo.java:326) at org.scijava.module.DefaultModuleService.createModule(DefaultModuleService.java:167) at org.scijava.module.DefaultModuleService.run(DefaultModuleService.java:206) at org.scijava.module.DefaultModuleService.run(DefaultModuleService.java:197) at org.scijava.module.DefaultModuleService.run(DefaultModuleService.java:182) at org.scijava.menu.ShadowMenu.run(ShadowMenu.java:335) at org.scijava.ui.swing.menu.AbstractSwingMenuCreator$1.actionPerformed(AbstractSwingMenuCreator.java:169) ... now have 1 CountyThingies Breakpoint reached at sc.iview.commands.add.AddBox.(AddBox.kt:75) Breakpoint reached at sc.iview.commands.add.AddBox.(AddBox.kt:75) at java.lang.invoke.DirectMethodHandle$Holder.newInvokeSpecial(DirectMethodHandle$Holder:-1) at java.lang.invoke.Invokers$Holder.invokeExact_MT(Invokers$Holder:-1) at jdk.internal.reflect.DirectConstructorHandleAccessor.invokeImpl(DirectConstructorHandleAccessor.java:86) at jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62) at java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:502) at java.lang.reflect.ReflectAccess.newInstance(ReflectAccess.java:128) at jdk.internal.reflect.ReflectionFactory.newInstance(ReflectionFactory.java:304) at java.lang.Class.newInstance(Class.java:725) at org.scijava.command.CommandModuleItem.getDefaultValue(CommandModuleItem.java:168) at org.scijava.module.DefaultModuleService.getDefaultValue(DefaultModuleService.java:321) at org.scijava.module.process.DefaultValuePreprocessor.assignDefaultValue(DefaultValuePreprocessor.java:76) at org.scijava.module.process.DefaultValuePreprocessor.process(DefaultValuePreprocessor.java:61) at org.scijava.module.ModuleRunner.preProcess(ModuleRunner.java:103) at org.scijava.module.ModuleRunner.run(ModuleRunner.java:154) at org.scijava.module.ModuleRunner.call(ModuleRunner.java:125) at org.scijava.module.ModuleRunner.call(ModuleRunner.java:64) at org.scijava.thread.DefaultThreadService.lambda$wrap$2(DefaultThreadService.java:247) at org.scijava.thread.DefaultThreadService$$Lambda/0x0000000800f2ac68.call(Unknown Source:-1) at java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:317) at java.util.concurrent.FutureTask.run(FutureTask.java:-1) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642) at java.lang.Thread.runWith(Thread.java:1596) at java.lang.Thread.run(Thread.java:1583) now have 2 CountyThingies Breakpoint reached at sc.iview.commands.add.AddBox.(AddBox.kt:75) Breakpoint reached at sc.iview.commands.add.AddBox.(AddBox.kt:75) at java.lang.invoke.DirectMethodHandle$Holder.newInvokeSpecial(DirectMethodHandle$Holder:-1) at java.lang.invoke.Invokers$Holder.invokeExact_MT(Invokers$Holder:-1) at jdk.internal.reflect.DirectConstructorHandleAccessor.invokeImpl(DirectConstructorHandleAccessor.java:86) at jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62) at java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:502) at java.lang.reflect.ReflectAccess.newInstance(ReflectAccess.java:128) at jdk.internal.reflect.ReflectionFactory.newInstance(ReflectionFactory.java:304) at java.lang.Class.newInstance(Class.java:725) at org.scijava.command.CommandModuleItem.getDefaultValue(CommandModuleItem.java:168) at org.scijava.module.DefaultModuleService.getDefaultValue(DefaultModuleService.java:321) at org.scijava.module.process.DefaultValuePreprocessor.assignDefaultValue(DefaultValuePreprocessor.java:76) at org.scijava.module.process.DefaultValuePreprocessor.process(DefaultValuePreprocessor.java:61) at org.scijava.module.ModuleRunner.preProcess(ModuleRunner.java:103) at org.scijava.module.ModuleRunner.run(ModuleRunner.java:154) at org.scijava.module.ModuleRunner.call(ModuleRunner.java:125) at org.scijava.module.ModuleRunner.call(ModuleRunner.java:64) at org.scijava.thread.DefaultThreadService.lambda$wrap$2(DefaultThreadService.java:247) at org.scijava.thread.DefaultThreadService$$Lambda/0x0000000800f2ac68.call(Unknown Source:-1) at java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:317) at java.util.concurrent.FutureTask.run(FutureTask.java:-1) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642) at java.lang.Thread.runWith(Thread.java:1596) at java.lang.Thread.run(Thread.java:1583) now have 3 CountyThingies Breakpoint reached at sc.iview.commands.add.AddBox.(AddBox.kt:75) Breakpoint reached at sc.iview.commands.add.AddBox.(AddBox.kt:75) at java.lang.invoke.DirectMethodHandle$Holder.newInvokeSpecial(DirectMethodHandle$Holder:-1) at java.lang.invoke.Invokers$Holder.invokeExact_MT(Invokers$Holder:-1) at jdk.internal.reflect.DirectConstructorHandleAccessor.invokeImpl(DirectConstructorHandleAccessor.java:86) at jdk.internal.reflect.DirectConstructorHandleAccessor.newInstance(DirectConstructorHandleAccessor.java:62) at java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:502) at java.lang.reflect.ReflectAccess.newInstance(ReflectAccess.java:128) at jdk.internal.reflect.ReflectionFactory.newInstance(ReflectionFactory.java:304) at java.lang.Class.newInstance(Class.java:725) at org.scijava.command.CommandModuleItem.getDefaultValue(CommandModuleItem.java:168) at org.scijava.module.DefaultModuleService.getDefaultValue(DefaultModuleService.java:321) at org.scijava.module.process.DefaultValuePreprocessor.assignDefaultValue(DefaultValuePreprocessor.java:76) at org.scijava.module.process.DefaultValuePreprocessor.process(DefaultValuePreprocessor.java:61) at org.scijava.module.ModuleRunner.preProcess(ModuleRunner.java:103) at org.scijava.module.ModuleRunner.run(ModuleRunner.java:154) at org.scijava.module.ModuleRunner.call(ModuleRunner.java:125) at org.scijava.module.ModuleRunner.call(ModuleRunner.java:64) at org.scijava.thread.DefaultThreadService.lambda$wrap$2(DefaultThreadService.java:247) at org.scijava.thread.DefaultThreadService$$Lambda/0x0000000800f2ac68.call(Unknown Source:-1) at java.util.concurrent.FutureTask.run$$$capture(FutureTask.java:317) at java.util.concurrent.FutureTask.run(FutureTask.java:-1) at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1144) at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:642) at java.lang.Thread.runWith(Thread.java:1596) at java.lang.Thread.run(Thread.java:1583) now have 4 CountyThingies ```

One idea to fix this would be to infer all of a command's default values the first time an instance is created as a module. Because an instance of the command is instantiated immediately: the one that is actually going to get run. The later ones are just throwaways. But if upon first instantiation, we inspected the instance's field values immediately and cached them into the CommandInfo itself, then we'd be able to reuse those values from the cache whenever getDefaultValue() is subsequently called, without instantiating throwaway instances.