Home > Back-end >  Why isn't this multithreading efficient in the for loop in this code?
Why isn't this multithreading efficient in the for loop in this code?

Time:06-09

I want to use multithreading to speed up queries in a for loop code in Spring.

Some people say that: the thread pool get method is Blocking methods, so it is no different from writing without threads.

So how does this code work with thread acceleration in a Java loop?

Some languages, such as C# ,JS , use await remoteCall(id), use 'await' Whether the same problem exists ?

@Data
class ResultDto {
    private BaseData baseData;
}
@Data
public class BaseData {
    public String baseInfo;
    public String remoteInfo;
}
ExecutorService exector = Executors.newCachedThreadPool();

public List<ResultDto> queryAll(List<String> ids) throws ExecutionException, InterruptedException {

    List<ResultDto> res = new ArrayList<>();

    for (String id : ids) {
        ResultDto resultDto = new ResultDto();
        BaseData baseData = new BaseData();
        baseData.setBaseInfo("baseData"   id);

        //using thread  blocking ?
        String remoteResult = exector.submit(() -> remoteCall(id)).get();

        baseData.setRemoteInfo(remoteResult);
        resultDto.setBaseData(baseData);
        res.add(resultDto);
    }

    return res;
}

String remoteCall(String id) {
    return " httpUtils.get()"   id;
}

With my limited experience with multithread Programing, my code looks ugly. How to improve it? Please help me rewrite the code below.

-------------------Edit----------------------------

Thank you all for your help.Now the code is much prettier. I am learning CompletableFuture by looking up the JDK 8 document.

I've changed the code, and now the only question is how do I get the ID in second map ?

Is there any way to pass an id param in java? (Or MAYBE I should change the ResultDto class and add an ID field ?)

Since baseInfo and remoteInfo correspond one to one in the ResultDto class, will I get data confusion if I write this way?

public List<ResultDto> queryAll2(List<String> ids) {

    return ids.stream().map(id -> {
                ResultDto resultDto = new ResultDto();
                BaseData baseData = new BaseData();
                baseData.setBaseInfo("baseData"   id);
                resultDto.setBaseData(baseData);
                return resultDto;
            }).collect(Collectors.toList())
            .stream()
            //How to get id param  ?
            .map(resultDto -> CompletableFuture.supplyAsync(() -> doWork(resultDto, "id ? "), executor))
            .collect(Collectors.toList())
            .stream()
            .map(CompletableFuture::join)
            .collect(Collectors.toList());


}

private ResultDto doWork(ResultDto resultDto, String id) {
    String remoteCall = this.remoteCall(id);
    resultDto.getBaseData().setRemoteInfo(remoteCall);
    return resultDto;
}

Thank you again.

CodePudding user response:

Do something like this.

ExecutorService exector = Executors.newCachedThreadPool();

public List<ResultDto> queryAll(List<String> ids) throws ExecutionException, InterruptedException {

    List<CompletableFuture> cfs = new ArrayList<>(ids.size());
    for (String id : ids) {
      CompletableFuture cf = Completableuture.supplyAsync(() -> getResult(id), exector);
      cfs.add(cf);
    }
    CompletableFuture allOfThem = CompletableFuture.allOf(cfs.toArray(new CompletableFuture[0]);
 
    CompletableFuture<List<ResultDto>> allCompletableFutures = allOfThem .thenApply(future -> {
    return cfs.stream()
            .map(completableFuture -> completableFuture.join())
            .collect(Collectors.toList());
});
    return allCompletableFutures.get();
}

ResultDto getResult(String id) {
    String remoteResult = " httpUtils.get()"   id;
    BaseData baseData = new BaseData();
    baseData.setBaseInfo("baseData"   id);
    baseData.setRemoteInfo(remoteResult);
    ResultDto resultDto = new ResultDto();
    resultDto.setBaseData(baseData);
    return resultDto;
}

This will give you a non-blocking solution (it will only block in the end).

Or you can make it even easier and just use a parallelStream and use the default fork-join pool.

public List<ResultDto> queryAll(List<String> ids) throws ExecutionException, InterruptedException {
    return ids.parallelStream().map(id -> getResult(id)).collect(Collectors.toList());
}

ResultDto getResult(String id) {
    String remoteResult = " httpUtils.get()"   id;
    BaseData baseData = new BaseData();
    baseData.setBaseInfo("baseData"   id);
    baseData.setRemoteInfo(remoteResult);
    ResultDto resultDto = new ResultDto();
    resultDto.setBaseData(baseData);
    return resultDto;
}

You can probably decide which is better to read...

CodePudding user response:

Some people say that: the thread pool get method is Blocking methods, so it is no different from writing without threads.

They are correct. If you call get() at that point, it immediately blocks until the that particular task has completed. So the 2nd task isn't submitted until the first one completes ... and so on.

What you need to do is submit all of the tasks before calling get(). Something like the following pseudo-code.

List<Future> futures
for each id in ids:
   futures.add(executor.submit(...))

for each future in futures:
   result = future.get()
   results.add(process(result))

If you can arrange that each task does the processing of its result, you potentially get more parallelism.

As noted, you could use CompleteableFuture.allOf instead of the second loop.

  • Related