GossipScore refactor #247
GossipScore refactor #247
Conversation
- extract 2 interfaces GossipRouterEventListener and GossipScore - Make GossipScore to operate with PeerId instead of PeerHandler
…erHandler - P2PService: hide mutable collections, expose immutable - P2PService: add peerIdToPeerHandlerMap to have optimized access to PeerHandler by its PeerId
| open val score: GossipScore by lazy { | ||
| DefaultGossipScore(scoreParams, executor, curTimeMillis).also { | ||
| eventBroadcaster.listeners += it | ||
| } | ||
| } |
There was a problem hiding this comment.
I'm guessing there's some fun kotlin magic that means this works even though eventBroadcaster is only defined and initialised after this?
It would probably make me feel better to just move the val eventBroadcaster = GossipRouterEventBroadcaster() line above this one though. :) I'm a simple Java programmer...
There was a problem hiding this comment.
It works just because the score in lazy initialized: the code inside lazy block is executed on the first score usage
TBH I was trying to invent 2-in-1 'builder+class' pattern with Kotlin but it's far from what it's intended to be and tends to be spaghetti-code pattern
There was a problem hiding this comment.
Was trying to refactor that quickly with the classic Builder pattern but seemed to open a can of worms so it's definitely worth a separate PR
There was a problem hiding this comment.
ah right - I hadn't spotted the lazy there at all, mostly because it's a kotlin feature so I'm just not used to it. I'm fine leaving it as-is but sounds like you're not happy with the pattern anyway so go with what you think.
GossipScoreconsumes significant CPU time under high network load.This is initial refactoring before optimizations
GossipRouter.scoreproperty overridableGossipRouterEventListenerandGossipScore. Rename implementation toDefaultGossipScorePeerIdinstead ofPeerHandlerPeerHandleraddP2PService:.peerIdToPeerHandlerMapP2PService: hide mutable collections, expose immutable