[Improvement] No need to use synchronized lock of the method scope when getting client

[zuston]

What changes were proposed in this pull request?

[Improvement] No need to use synchronized lock of the method scope when getting client

Why are the changes needed?

Avoid unnecessary lock to improve performance.

Does this PR introduce any user-facing change?

No

How was this patch tested?

No need.

[jerqi]

I can't tell the difference, we use the synchronized lock. Before this patch, we lock the method. After the patch, we lock the class. I think they have the same performance.

[zuston]

The lock is just to avoid concurrency conflict on initialization of client. Once Instance is initialized, no need to lock. Right? @jerqi

[jerqi]

The lock is just to avoid concurrency conflict on initialization of client. Once Instance is initialized, no need to lock. Right? @jerqi

I got your point. But I prefer use LazyHolder
https://github.com/apache/incubator-uniffle/blob/20d39e8455788ce27b97c9a63031d8c42cf64e5f/server/src/main/java/org/apache/uniffle/server/storage/StorageManagerFactory.java#L25
More information, u can see
https://stackoverflow.com/questions/70819318/lazyholder-uses-a-static-class-why-is-it-called-lazyholder

[zuston]

Yes. Double-ckeck lock / enum / Inner class are all optional for this requirement. If u prefer inner class. i will update new one later.

[jerqi]

Yes. Double-ckeck lock / enum / Inner class are all optional for this requirement. If u prefer inner class. i will update new one later.

We should unify our style. We use inner class in the other places.

[codecov-commenter]

Codecov Report

Merging #82 (f658a69) into master (aa18be0) will decrease coverage by 0.08%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master      #82      +/-   ##
============================================
- Coverage     56.39%   56.31%   -0.09%     
- Complexity     1173     1174       +1     
============================================
  Files           149      149              
  Lines          7953     7977      +24     
  Branches        761      765       +4     
============================================
+ Hits           4485     4492       +7     
- Misses         3226     3242      +16     
- Partials        242      243       +1     

Impacted Files	Coverage Δ
.../apache/uniffle/coordinator/CoordinatorServer.java	68.67% <0.00%> (-2.22%)	⬇️
...org/apache/uniffle/server/ShuffleFlushManager.java	76.70% <0.00%> (-1.71%)	⬇️
...e/uniffle/server/storage/SingleStorageManager.java	65.57% <0.00%> (-1.64%)	⬇️
.../java/org/apache/uniffle/server/ShuffleServer.java	63.41% <0.00%> (-1.30%)	⬇️
...he/uniffle/client/impl/ShuffleWriteClientImpl.java	25.95% <0.00%> (-0.10%)	⬇️
...he/uniffle/coordinator/CoordinatorGrpcService.java	2.36% <0.00%> (-0.06%)	⬇️
...java/org/apache/uniffle/common/rpc/GrpcServer.java	0.00% <0.00%> (ø)
.../hadoop/mapreduce/task/reduce/RssEventFetcher.java	88.57% <0.00%> (+1.68%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us.

[jerqi]

LGTM