0

I am using spring boot (generated by JHipster) with some services that are using a List as field.
I was wondering wether I should empty or nullify this list at the end of the methods that use it.
Is there any impact on JVM memory ?
Considering this method can be called 100x + per day, and considering that each user has its own execution context so fields are not erasing previous ones as far as I understand.

Example :

package fr.vyvcareit.poccarto.service;

//imports

@Service
@Transactional
public class SiteService {

    //Liste temporaire pour repérer les doublons
    private List<String> siteCodesDansImport ;

    public SiteService() {  }

    public void preImportSiteError(List<Object> rows) {

        this.siteCodesDansImport = new ArrayList<String>();

        for (int i = 0; i < rows.size(); i++) {      
            checkSiteCode(int num_ligne, HashMap row);
        }
        // I'm all done, I do not need this.siteCodesDansImport anymore...
        this.siteCodesDansImport=null; // => Is this line important for java memory ???

    }

    private void checkSiteCode(int num_ligne, HashMap row){
        ...
        siteCodesDansImport.add(site_code);
        ...
    }
}

Any help would be appreciated !

luboskrnac
  • 23,973
  • 10
  • 81
  • 92
tacou_st
  • 109
  • 1
  • 9
  • 2
    Why don't you make it a local variable inside the method? – jonrsharpe Aug 30 '18 at 07:22
  • 3
    If this `@Service` is to be used to serve multiple concurrent requests, then you are already using it badly because you treat it as though it were request-scoped. – ernest_k Aug 30 '18 at 07:22
  • From the current code shared looks like you don't need to have `siteCodesDansImport ` defined at class level – Himanshu Bhardwaj Aug 30 '18 at 07:30
  • @jonrsharpe, I edit the code to explain why I need class variable – tacou_st Aug 30 '18 at 07:46
  • No you don't, just pass it as a parameter. – jonrsharpe Aug 30 '18 at 07:47
  • @ernest_k, I don't understand : If there is 2 simultaneous calls, are we agree that this.siteCodesDansImport will be different and independant ? – tacou_st Aug 30 '18 at 07:49
  • @tacou_st If objects of this class serve multiple concurrent users, then the instance field is there only to introduce trouble (it doesn't even represent necessary cross-request state). Different threads will be manipulating the list simultaneously, resulting inconsistent results. – ernest_k Aug 30 '18 at 07:53
  • @jonrsharpe, ok thank you, this is working well ! But just for sights, from my previous implementation, was there a potential memory issue ? – tacou_st Aug 30 '18 at 07:54
  • @ernest_k Wow, thank you for pointing this, I completely misunderstood how services are instantiated and used. Need some readings... – tacou_st Aug 30 '18 at 08:01
  • @tacou_st I'm not very familiar with spring. But looks like luboskrnac's answer sheds more light. But beside that, I don't think that setting the value to null makes much difference for the singleton service. – ernest_k Aug 30 '18 at 08:05

1 Answers1

3

By default, Spring beans has singleton scope. That means your service has single instance per application. Having fields on singleton beans is not thread safe and should be avoided.

Every new request will override state of your collection and requests will be affecting each other states, which will lead to unpredictable behavior.

Never ever store state into field variables of singleton beans.

For your collection just use local variable. Pass it around via method parameters (e.g. into checkSiteCode). At the end execution, you don't need to set it to null. Java GC will take care of it.

luboskrnac
  • 23,973
  • 10
  • 81
  • 92