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.

Sign up for GitHub

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

Jump to bottom

Commands get instantiated multiple times per run #476

Open
ctrueden opened this issue Mar 19, 2024 · 0 comments
Open

Commands get instantiated multiple times per run #476

ctrueden opened this issue Mar 19, 2024 · 0 comments
Labels
bug

Comments

@ctrueden
Copy link
Member

ctrueden commented Mar 19, 2024 •
edited
Loading

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.<init>(AddBox.kt:75)
Breakpoint reached
    at sc.iview.commands.add.AddBox.<init>(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.<init>(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.<init>(AddBox.kt:75)
Breakpoint reached
    at sc.iview.commands.add.AddBox.<init>(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.<init>(AddBox.kt:75)
Breakpoint reached
    at sc.iview.commands.add.AddBox.<init>(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.<init>(AddBox.kt:75)
Breakpoint reached
    at sc.iview.commands.add.AddBox.<init>(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.
@ctrueden ctrueden added the bug label Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@ctrueden