KAFKA-16879: SystemTime should use singleton mode #16179
Conversation
| public interface Time { | ||
|
|
||
| Time SYSTEM = new SystemTime(); | ||
| Time SYSTEM = SystemTime.getInstance(); |
There was a problem hiding this comment.
Hey, is there a reason this isn't the canonical singleton instance? Why would we want two different ways to get the singleton Time.SYSTEM and SystemTime.getInstance()?
There was a problem hiding this comment.
You are right, I should deprecate Time SYSTEM this property, and use SystemTime.getInstance() to instead it.
There was a problem hiding this comment.
I would be inclined to keep the existing Time.SYSTEM and avoid the deprecation. SystemTime can then be package-local entirely.
| * use the static inner class to implement the singleton pattern | ||
| * to avoid the overhead of synchronization |
There was a problem hiding this comment.
Can you explain this comment? I don't know what it's referring to, as none of this code is synchronized.
The static inner class seems to be a lazy initialization trick, which is ineffective here (due to Time.SYSTEM) and unneeded (since the SystemTime constructor is very cheap).
There was a problem hiding this comment.
True, SystemTime is very cheap, but I think that don't use double-checked locking to implement the singleton pattern, maybe I should remove this comment is more better.
2 participants
https://issues.apache.org/jira/browse/KAFKA-16879
I use the static inner class to implement to singleton pattern.
Committer Checklist (excluded from commit message)