*: optimize memory usage #8164

rleungx · 2024-05-10T08:40:55Z

What problem does this PR solve?

Issue Number: Ref #7897

What is changed and how does it work?

Check List

Tests

Manual test
compared with beb91c1

goos: linux
goarch: amd64
pkg: github.com/tikv/pd/server/cluster
cpu: Intel(R) Xeon(R) Gold 6240 CPU @ 2.60GHz
                         │   old.txt   │               new.txt                │
                         │   sec/op    │    sec/op     vs base                │
HandleRegionHeartbeat-72   5.049µ ± 9%   3.248µ ± 15%  -35.67% (p=0.000 n=10)

                         │  old.txt   │              new.txt               │
                         │    B/op    │    B/op     vs base                │
HandleRegionHeartbeat-72   989.5 ± 3%   571.0 ± 4%  -42.29% (p=0.000 n=10)

                         │  old.txt   │              new.txt               │
                         │ allocs/op  │ allocs/op   vs base                │
HandleRegionHeartbeat-72   25.00 ± 0%   14.00 ± 7%  -44.00% (p=0.000 n=10)

Release note

None.

ti-chi-bot · 2024-05-10T08:40:57Z

[REVIEW NOTIFICATION]

This pull request has been approved by:

nolouch

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

codecov · 2024-05-11T03:56:52Z

Codecov Report

Attention: Patch coverage is 92.35669% with 12 lines in your changes missing coverage. Please review.

Project coverage is 77.28%. Comparing base (14c68f6) to head (b7ee174).

❗ Current head b7ee174 differs from pull request most recent head 32ce8d9

Please upload reports for the commit 32ce8d9 to get more accurate results.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #8164      +/-   ##
==========================================
- Coverage   77.42%   77.28%   -0.14%     
==========================================
  Files         471      470       -1     
  Lines       61405    61334      -71     
==========================================
- Hits        47540    47400     -140     
- Misses      10308    10358      +50     
- Partials     3557     3576      +19

Flag	Coverage Δ
unittests	`77.28% <92.35%> (-0.14%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

HuSharp · 2024-05-11T06:18:53Z

pkg/utils/grpcutil/grpcutil.go

-	}
-	if t, ok := md[ForwardMetadataKey]; ok {
-		return t[0]
+	s := metadata.ValueFromIncomingContext(ctx, ForwardMetadataKey)


Do we need to change other FromIncomingContext?

I prefer not for now since it's still in experimental state.

HuSharp · 2024-05-11T09:20:38Z

pkg/ratelimit/runner.go

@@ -197,7 +202,7 @@ func (cr *ConcurrentRunner) RunTask(ctx context.Context, f func(context.Context)
 		}
 		task.submittedAt = time.Now()
 		cr.pendingTasks = append(cr.pendingTasks, task)
-		cr.pendingTaskCount[taskOpts.TaskName]++


Is taskName a required field? If so, should it be in the task struct?

Signed-off-by: Ryan Leung <[email protected]>

nolouch

lgtm

nolouch · 2024-05-31T07:02:46Z

server/cluster/cluster_test.go

 	// Reset timer after setup
 	b.ResetTimer()
 	// Run HandleStatsAsync b.N times
 	for i := 0; i < b.N; i++ {
 		cluster.HandleStatsAsync(c, region)
 	}
 }
+
+func BenchmarkHandleRegionHeartbeat(b *testing.B) {


maybe we can run some benchmarks daily.

Signed-off-by: Ryan Leung <[email protected]>

ti-chi-bot · 2024-06-13T07:25:46Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: HuSharp, nolouch

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [HuSharp,nolouch]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2024-06-13T07:25:47Z

[LGTM Timeline notifier]

Timeline:

2024-06-13 07:25:47.069633931 +0000 UTC m=+622301.122945839: ☑️ agreed by HuSharp.

rleungx · 2024-06-13T09:10:02Z

/merge

ti-chi-bot · 2024-06-13T09:10:05Z

@rleungx: We have migrated to builtin LGTM and approve plugins for reviewing.

👉 Please use /approve when you want approve this pull request.

The changes announcement: Proposal: Strengthen configuration change approval.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot · 2024-06-13T09:10:20Z

@rleungx: Your PR was out of date, I have automatically updated it for you.

If the CI test fails, you just re-trigger the test that failed and the bot will merge the PR for you after the CI passes.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the ti-community-infra/tichi repository.

ti-chi-bot bot added the release-note-none Denotes a PR that doesn't merit a release note. label May 10, 2024

ti-chi-bot bot requested review from HuSharp and nolouch May 10, 2024 08:41

ti-chi-bot bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 10, 2024

rleungx force-pushed the memory branch 4 times, most recently from b697eed to 91fbb40 Compare May 11, 2024 03:45

nolouch approved these changes May 11, 2024

View reviewed changes

ti-chi-bot bot added the status/LGT1 Indicates that a PR has LGTM 1. label May 11, 2024

HuSharp reviewed May 11, 2024

View reviewed changes

rleungx force-pushed the memory branch from 85ba1d6 to 89eb56d Compare May 28, 2024 07:11

ti-chi-bot bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 28, 2024

rleungx force-pushed the memory branch from 89eb56d to 49aca87 Compare May 28, 2024 07:27

rleungx added 5 commits May 28, 2024 16:36

reduce handle region heartbeat memory

54771cf

Signed-off-by: Ryan Leung <[email protected]>

bench

ce4b5e4

Signed-off-by: Ryan Leung <[email protected]>

fix

1e85e73

Signed-off-by: Ryan Leung <[email protected]>

sort once

0f57ad2

Signed-off-by: Ryan Leung <[email protected]>

fix

eebcdd9

Signed-off-by: Ryan Leung <[email protected]>

rleungx force-pushed the memory branch from b26849f to eebcdd9 Compare May 28, 2024 08:37

fix

83b2adf

Signed-off-by: Ryan Leung <[email protected]>

nolouch approved these changes May 31, 2024

View reviewed changes

stable tests

b7ee174

Signed-off-by: Ryan Leung <[email protected]>

rleungx requested a review from HuSharp June 4, 2024 06:03

nolouch mentioned this pull request Jun 5, 2024

Enhance heartbeat processing capabilities #7897

Closed

7 tasks

HuSharp approved these changes Jun 13, 2024

View reviewed changes

ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Jun 13, 2024

rleungx added lgtm status/LGT2 Indicates that a PR has LGTM 2. and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. status/LGT1 Indicates that a PR has LGTM 1. labels Jun 13, 2024

ti-chi-bot bot added status/LGT1 Indicates that a PR has LGTM 1. and removed status/LGT2 Indicates that a PR has LGTM 2. labels Jun 13, 2024

Merge branch 'master' into memory

32ce8d9

ti-chi-bot bot added the dco-signoff: yes Indicates the PR's author has signed the dco. label Jun 13, 2024

ti-chi-bot bot merged commit 3e34b80 into tikv:master Jun 13, 2024
16 checks passed

rleungx deleted the memory branch June 13, 2024 09:36

This was referenced Jun 27, 2024

statistics: meet data race when check IsRegionHot #8335

Closed

statistics: fix data race in IsRegionHot #8336

Merged

ti-chi-bot mentioned this pull request Jun 27, 2024

statistics: fix data race in IsRegionHot (#8336) #8341

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: optimize memory usage #8164

*: optimize memory usage #8164

rleungx commented May 10, 2024 •

edited

Loading

ti-chi-bot bot commented May 10, 2024 •

edited

Loading

codecov bot commented May 11, 2024 •

edited

Loading

HuSharp May 11, 2024

rleungx May 11, 2024

HuSharp May 11, 2024

nolouch left a comment

nolouch May 31, 2024

ti-chi-bot bot commented Jun 13, 2024

ti-chi-bot bot commented Jun 13, 2024

rleungx commented Jun 13, 2024

ti-chi-bot bot commented Jun 13, 2024

ti-chi-bot bot commented Jun 13, 2024

*: optimize memory usage #8164

*: optimize memory usage #8164

Conversation

rleungx commented May 10, 2024 • edited Loading

What problem does this PR solve?

What is changed and how does it work?

Check List

Release note

ti-chi-bot bot commented May 10, 2024 • edited Loading

codecov bot commented May 11, 2024 • edited Loading

Codecov Report

HuSharp May 11, 2024

Choose a reason for hiding this comment

rleungx May 11, 2024

Choose a reason for hiding this comment

HuSharp May 11, 2024

Choose a reason for hiding this comment

nolouch left a comment

Choose a reason for hiding this comment

nolouch May 31, 2024

Choose a reason for hiding this comment

ti-chi-bot bot commented Jun 13, 2024

ti-chi-bot bot commented Jun 13, 2024

[LGTM Timeline notifier]

rleungx commented Jun 13, 2024

ti-chi-bot bot commented Jun 13, 2024

ti-chi-bot bot commented Jun 13, 2024

rleungx commented May 10, 2024 •

edited

Loading

ti-chi-bot bot commented May 10, 2024 •

edited

Loading

codecov bot commented May 11, 2024 •

edited

Loading