-
Notifications
You must be signed in to change notification settings - Fork 148
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.
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
[Improvement] Make codec to be a singleton. #389
Comments
Will it be thread safe? |
Yes. In fact, the underlying codec is also a singleton. |
cc @zuston |
The zstd and snappy codec looks OK of singleton. But LZ4Codec looks unsupported. |
It seems that LZ4Codec is also singleton |
I'm a newbie and I want to try to solve this problem, is it possible? |
I assign this issue to you. You can have a try. |
lz4/lz4-java#82 Lz4 is thread safe. So it could be an singleton. |
Do you need any help from me? |
|
@tobehardest Do you know Singleton pattern? There is an example https://github.com/apache/incubator-uniffle/blob/master/internal-client/src/main/java/org/apache/uniffle/client/factory/ShuffleManagerClientFactory.java . You can refer to it. The document is also useful for newbie. https://github.com/apache/incubator-uniffle/blob/master/CONTRIBUTING.md |
### What changes were proposed in this pull request? Codec will be created for many times. We should make codec to be a singleton. I make codec to be a singleton. ### Why are the changes needed? Fix: #389 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? UT Co-authored-by: tobehardest <cvchen587@163.com>
Code of Conduct
Search before asking
What would you like to be improved?
Codec will be created for many times. We should make codec to be a singleton.
incubator-uniffle/common/src/main/java/org/apache/uniffle/common/compression/Codec.java
Lines 29 to 40 in 8847ece
How should we improve?
Make codec to be a singleton.
Are you willing to submit PR?
The text was updated successfully, but these errors were encountered: