-
Notifications
You must be signed in to change notification settings - Fork 42
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
Set tipChanged subject's buffer size to 1 #2245
Conversation
@moreal Tests don't seem to finish 😲 |
@moreal Could you take a look at this lint error? (https://github.com/planetarium/NineChronicles.Headless/actions/runs/6193775290/job/16815783641?pr=2245#step:4:34) |
b4c7d48
to
92011e8
Compare
I fixed it! |
92011e8
to
24c9424
Compare
@moreal https://github.com/planetarium/NineChronicles.Headless/actions/runs/6195442164/job/16821210098?pr=2245#step:5:5294 Not sure why this keeps failing 😅 |
1d4821b
to
d692252
Compare
This PR has Quantification details
Why proper sizing of changes matters
Optimal pull request sizes drive a better predictable PR flow as they strike a
What can I do to optimize my changes
How to interpret the change counts in git diff output
Was this comment helpful? 👍 :ok_hand: :thumbsdown: (Email) |
@area363 Thanks for letting me know that 🙏🏻 I think it is a timing issue between the subscription timing and the reactive processor's timing ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
This pull request updates the buffer size of
subscription.tipChanged
API's subject to 1. Without the limitation, theReplaySubject
usesint.MaxValue
as its buffer size. It is very useless to providetipChanged
subscription feature. It just causes too many subscription responses and uses too much memory.When I tested with launcher, it works well.