Skip to content
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

perf(client): optimize with atomic.Int64 for lastSubscribeAt in pkg/varlog.(subscriber) #720

Merged
merged 1 commit into from
Mar 5, 2024

Conversation

ijsong
Copy link
Member

@ijsong ijsong commented Feb 25, 2024

What this PR does

Switches the type of pkg/varlog.(subscriber).lastSubscribeAt from
atomic.Value to atomic.Int64 to eliminate heap escapes. The frequent updates
with new time.Time values to lastSubscribeAt previously led to significant
heap allocation due to heap escape. By adopting atomic.Int64, we can prevent
heap escape, thereby reducing heap allocations and enhancing performance.

@ijsong ijsong self-assigned this Feb 25, 2024
@ijsong ijsong marked this pull request as draft February 25, 2024 13:06
@ijsong ijsong force-pushed the perf_reuse_subscriberesponse branch from 00ab217 to 4f7103f Compare February 25, 2024 13:15
@ijsong ijsong force-pushed the perf_subscriber_lastsubscribeat branch from a66b30e to e1ef1ad Compare February 25, 2024 13:15
@ijsong ijsong force-pushed the perf_reuse_subscriberesponse branch from 4f7103f to cc39617 Compare February 25, 2024 14:54
@ijsong ijsong force-pushed the perf_subscriber_lastsubscribeat branch from e1ef1ad to 416e9f9 Compare February 25, 2024 14:54
Copy link

codecov bot commented Feb 25, 2024

Codecov Report

Attention: Patch coverage is 42.85714% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 49.75%. Comparing base (890508d) to head (a58015b).
Report is 1 commits behind head on main.

❗ Current head a58015b differs from pull request most recent head 617bd9d. Consider uploading reports for the commit 617bd9d to get more accurate results

Files Patch % Lines
pkg/varlog/subscribe.go 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #720      +/-   ##
==========================================
+ Coverage   49.67%   49.75%   +0.08%     
==========================================
  Files         235      235              
  Lines       20717    20718       +1     
==========================================
+ Hits        10291    10309      +18     
+ Misses       9783     9773      -10     
+ Partials      643      636       -7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ijsong ijsong force-pushed the perf_reuse_subscriberesponse branch from cc39617 to f59cbe0 Compare February 26, 2024 14:43
@ijsong ijsong force-pushed the perf_subscriber_lastsubscribeat branch from 416e9f9 to e74d9c6 Compare February 26, 2024 14:44
@ijsong ijsong force-pushed the perf_reuse_subscriberesponse branch from f59cbe0 to 4baaff2 Compare February 28, 2024 00:23
@ijsong ijsong force-pushed the perf_subscriber_lastsubscribeat branch from e74d9c6 to 43444d0 Compare February 28, 2024 00:23
@ijsong ijsong force-pushed the perf_reuse_subscriberesponse branch from 4baaff2 to cd283ae Compare February 28, 2024 04:23
@ijsong ijsong force-pushed the perf_subscriber_lastsubscribeat branch from 43444d0 to c43198f Compare February 28, 2024 04:23
@ijsong ijsong force-pushed the perf_reuse_subscriberesponse branch from cd283ae to 0c1d77a Compare February 28, 2024 04:31
@ijsong ijsong force-pushed the perf_subscriber_lastsubscribeat branch from c43198f to a4f18ee Compare February 28, 2024 04:31
@ijsong ijsong marked this pull request as ready for review February 28, 2024 04:32
@ijsong ijsong force-pushed the perf_reuse_subscriberesponse branch from 0c1d77a to 578b928 Compare February 28, 2024 04:49
@ijsong ijsong force-pushed the perf_subscriber_lastsubscribeat branch from a4f18ee to 48a2b26 Compare February 28, 2024 04:49
@ijsong ijsong force-pushed the perf_reuse_subscriberesponse branch from 578b928 to 1b8f5a0 Compare March 5, 2024 07:02
@ijsong ijsong force-pushed the perf_subscriber_lastsubscribeat branch from 48a2b26 to a58015b Compare March 5, 2024 07:02
@ijsong
Copy link
Member Author

ijsong commented Mar 5, 2024

Merge activity

  • Mar 5, 4:41 AM EST: @ijsong started a stack merge that includes this pull request via Graphite.
  • Mar 5, 4:51 AM EST: Graphite rebased this pull request as part of a merge.
  • Mar 5, 4:58 AM EST: @ijsong merged this pull request with Graphite.

@ijsong ijsong force-pushed the perf_reuse_subscriberesponse branch from 1b8f5a0 to 890508d Compare March 5, 2024 09:42
Base automatically changed from perf_reuse_subscriberesponse to main March 5, 2024 09:50
…arlog.(subscriber)

Switches the type of `pkg/varlog.(subscriber).lastSubscribeAt` from
`atomic.Value` to `atomic.Int64` to eliminate heap escapes. The frequent updates
with new `time.Time` values to `lastSubscribeAt` previously led to significant
heap allocation due to heap escape. By adopting `atomic.Int64`, we can prevent
heap escape, thereby reducing heap allocations and enhancing performance.
@ijsong ijsong force-pushed the perf_subscriber_lastsubscribeat branch from a58015b to 617bd9d Compare March 5, 2024 09:50
@ijsong ijsong merged commit 6d2779e into main Mar 5, 2024
9 checks passed
@ijsong ijsong deleted the perf_subscriber_lastsubscribeat branch March 5, 2024 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants