9

In my web application, I have a background service. This service uses Generator class that contains an Engine class and an ExecutorService configured to use multiple threads and that accepts GeneratorTasks.

@Component
public class Generator {
    @Autowired
    private Engine heavyEngine;

    private ExecutorService exec = Executors.newFixedThreadPool(3);

    //I actually pass the singleton instance Generator class into the task.
    public void submitTask(TaskModel model, TaskCallback callback) {
        this.exec.submit(new GeneratorTask(model, this, callback));
    }
}

@Component
public class Engine {
    public Engine() {
        //time-consuming initialization code here
    }
}

public class GeneratorTask implements Callable<String> {
    public GeneratorTask(TaskModel m, Generator g, ReceiptCallback c) {
        this.m = m;
        this.generator = g;
        this.c = c;
    }

    public String call() throws Exception {
        //This actually calls the Engine class of the generator.
        //Maybe I should have passed the Engine itself?
        this.generator.runEngine(c);  
    }
}

The Engine class takes a long time to initialize so I ideally want to initialize it only once per thread. I can't just make it a singleton instance because the instance can't be shared across multiple threads (it relies on sequential processing). It's perfectly fine to reuse the instance though, after a processing task has completed.

I was thinking of making the private Engine heavyEngine variable a ThreadLocal variable. However, I'm also new to Spring so I was wondering if there might be another way to inject ThreadLocal variables using Spring annotations. I've looked at scoping the bean to request scope, but I'm not sure how I should go about it given my design.

Any guidance on how to improve my design would be appreciated.

Jensen Ching
  • 3,144
  • 4
  • 26
  • 42
  • Since `Engine` is autowired I assume you have declared it as a bean. By default Spring beans are singleton, so there is a good chance that you `Engine` is already a singleton... – a.b.d Sep 27 '12 at 03:23
  • Sorry I might have been unclear. My problem is, I can't make Engine a singleton because due to its processing logic, it can't be accessed by multiple concurrent threads. It's safe to reuse the Engine once a complete 'job' has been done, which is why I was thinking of making an instance per thread that uses it. – Jensen Ching Sep 27 '12 at 03:27
  • I understand, I was saying that with your current code Engine is already a singleton, even if it's not what you want it's what your code is actually doing. – a.b.d Sep 27 '12 at 03:31
  • Ah, I see. I already realized that- actually my current implementation is just using a `singleThreadExecutor` so I don't run into any concurrency issues. I'm just looking for other ways to implement a ThreadLocal-like behavior for this design. =) – Jensen Ching Sep 27 '12 at 03:40

3 Answers3

12

First of all abandon ThreadLocal - there is something scary in that class. What you need is just object pooling. It's not well known feature, but Spring supports this as well:

<bean id="engineProto" class="Engine" scope="prototype" lazy-init="true"/>

<bean id="engine" class="org.springframework.aop.framework.ProxyFactoryBean">
    <property name="targetSource">
        <bean class="org.springframework.aop.target.CommonsPoolTargetSource">
            <property name="targetClass" value="Engine"/>
            <property name="targetBeanName" value="engineProto"/>
            <property name="maxSize" value="3"/>
            <property name="maxWait" value="5000"/>
        </bean>
    </property>
</bean>

Now when you inject engine, you'll actually receive proxy object (Engine will need an interface) that will delegate all calls to free object in the pool. Pool size is configurable. Of course there is nothing preventing you from using ThreadLocalTargetSource which uses ThreadLocal instead of Commons Pool. Both approaches guarantee exclusive, thread safe access to Engine.

Finally you can use pooling manually (but the beauty of solution above is that it's completely transparent) or switch to EJBs, which are pooled by definition.

Tomasz Nurkiewicz
  • 334,321
  • 69
  • 703
  • 674
  • 1
    Classloader memory leaks are kinda scary. – Vedran May 28 '13 at 08:26
  • So you can inject the "engine" bean into anywhere expecting an Engine interface instance? Does it just create an anonymous class that inherits from the Engine interface that uses pooling behind-the-scenes? – guitar80 Dec 15 '16 at 00:27
5

FYI, Spring 3.0 and later includes a thread-backed Scope implementation, SimpleThreadScope.

In order to use it you need to register a custom scope:

<bean class="org.springframework.beans.factory.config.CustomScopeConfigurer">
    <property name="scopes">
        <map>
            <entry key="thread">
                <bean class="org.springframework.context.support.SimpleThreadScope" />
            </entry>
        </map>
    </property>
</bean>

And then to declare a thread-scoped bean:

<bean id="myBean" class="com.foo.MyBean" scope="thread">
    ...
</bean>
helmy
  • 9,068
  • 3
  • 32
  • 31
1

I would have created a factory for Engine and call it inside GeneratorTask. By this way you can remove the heavyEngine field inside Generator and the Generator constructor argument in GeneratorTask.
Then if you want to save the initialization time of Engine you can still declare it as a singleton but use the synchronized keyword on non thread safe methods.

public class Generator {    
    @Autowired private EngineFactory engineFactory;
    private ExecutorService exec = Executors.newFixedThreadPool(3);

    public void submitTask(TaskModel model, TaskCallback callback) {
        this.exec.submit(new GeneratorTask(engineFactory, model, callback));
    }
}

public class EngineFactory {
    @Autowired private Engine instance;

    public Engine getInstance() {
        return instance;
    }
}

public class Engine {
    public Engine() {
        //time-consuming initialization code here
    }

    public synchronized void runEngine() {
        // Do non thread safe stuf
    } 
}

public class GeneratorTask implements Callable<String> {
    public GeneratorTask(EngineFactory f, TaskModel m, ReceiptCallback c) {
        this.f = f;
        this.m = m;
        this.c = c;
    }

    public String call() throws Exception {
        Engine engine = f.getInstance();
        engine.runEngine();
        ... 
    }
}

There is probably a pure Spring way to pass the engine to the Callable but in this case the factory is good enough in my opinion.

a.b.d
  • 2,190
  • 3
  • 26
  • 26