profiling: add internal changes to support profiling of gRPC #3158

adtac · 2019-11-07T23:25:24Z

No description provided.

dfawley · 2019-11-11T18:25:42Z

We'll have to make vet.sh happy even though we only quasi-support appengine standard now. Can you add the exclude-appengine build tag to anything importing unsafe?

internal/profiling/profiling.go

adtac · 2019-11-14T01:06:58Z

Pushed V2 addressing review comments.

Ping @dfawley @easwars

dfawley · 2019-11-20T00:19:30Z

Travis is unhappy right now:

internal/profiling/profiling.go:149:13: undeclared name: goId (compile)
internal/profiling/profiling.go:149:3: unknown field GoId in struct literal (compile)
internal/profiling/profiling.go:149:13: undeclared name: goId (compile)
internal/profiling/profiling.go:149:3: unknown field GoId in struct literal (compile)
internal/profiling/profiling.go:149:13: undeclared name: goId (compile)
internal/profiling/profiling.go:149:3: unknown field GoId in struct literal (compile)

adtac · 2019-11-20T00:53:38Z

ugh, fixed (waiting for travis)

dfawley

Sending some partial comments (mostly nits) since more commits have been pushed and I don't want either one of us to have to deal with the github UI in this situation too much!

internal/profiling/buffer.go

adtac · 2019-11-20T20:48:42Z

@dfawley thanks for the review, addressed your comments and pushed V7, V8. (But don't bother with the actual commits; I imagine you're already using the Files Changed tab.)

internal/profiling/buffer.go

internal/profiling/buffer_test.go

internal/profiling/profiling.go

internal/profiling/profiling_test.go

adtac · 2019-11-22T20:32:37Z

@easwars thanks for the comments, pushed PATCH V9, PTAL.

dfawley · 2019-11-22T22:43:43Z

FYI travis is failing; it looks like your test needs to be conditional on appengine?

adtac · 2019-11-22T22:55:42Z

Ah yes, I forgot to add the build tag to the new tests. Pushed V10.

dfawley

Looks really good overall! Just a handful of comments, and I don't anticipate any more.

internal/profiling/buffer.go

internal/profiling/goid_modified.go

internal/profiling/profiling.go

1. revert lock-free stat/timer 2. use drainExecuting 3. review comments

1. go fmt

adtac · 2019-12-05T21:57:29Z

@dfawley, @menghanl thanks for the review! Updated the PR:

Rebased on to latest master.
reverted the lock-free stat/timer thing -- I found a data race in that approach with some additional testing. Apparently, dynamically resizing, lock-free arrays are apparently an area of active research for a reason :P
Addressed review comments.

internal/profiling/buffer/buffer.go

internal/profiling/profiling.go

internal/profiling/buffer/buffer_appengine.go

internal/profiling/profiling.go

adtac · 2019-12-10T21:58:42Z

@menghanl pushed V19 modifying CircularBuffer's documentation.

adtac mentioned this pull request Nov 7, 2019

[WIP] add profiling to gRPC #3130

Closed

dfawley added the Type: Performance Performance improvements (CPU, network, memory, etc) label Nov 11, 2019

dfawley added this to the 1.26 Release milestone Nov 11, 2019

easwars reviewed Nov 11, 2019

View reviewed changes

internal/profiling/profiling.go Outdated Show resolved Hide resolved

internal/profiling/profiling.go Outdated Show resolved Hide resolved

internal/profiling/profiling.go Outdated Show resolved Hide resolved

internal/profiling/profiling.go Outdated Show resolved Hide resolved

easwars reviewed Nov 11, 2019

View reviewed changes

internal/profiling/profiling.go Show resolved Hide resolved

easwars assigned adtac Nov 11, 2019

easwars assigned easwars and unassigned adtac Nov 15, 2019

adtac force-pushed the adtac/profiling-core branch 4 times, most recently from c5b9976 to 3e06a41 Compare November 16, 2019 00:15

adtac force-pushed the adtac/profiling-core branch from 89156b3 to 66c564b Compare November 20, 2019 00:52

adtac force-pushed the adtac/profiling-core branch from 5748f74 to ce15d1b Compare November 20, 2019 01:44

dfawley reviewed Nov 20, 2019

View reviewed changes

internal/profiling/buffer.go Outdated Show resolved Hide resolved

internal/profiling/buffer.go Outdated Show resolved Hide resolved

internal/profiling/buffer.go Outdated Show resolved Hide resolved

internal/profiling/buffer.go Outdated Show resolved Hide resolved

easwars removed their assignment Nov 20, 2019

dfawley self-assigned this Nov 21, 2019

easwars reviewed Nov 21, 2019

View reviewed changes

easwars assigned adtac and unassigned dfawley Nov 21, 2019

adtac force-pushed the adtac/profiling-core branch from 826aa4f to 4bc5eaa Compare November 22, 2019 20:32

dfawley reviewed Nov 22, 2019

View reviewed changes

Adhityaa Chandrasekar added 9 commits December 5, 2019 13:54

core: PATCH V6

2850cf1

core: PATCH V7

523dab3

core: PATCH V8

9851b3a

core: PATCH V9

47423cb

core: PATCH V10

525db95

core: PATCH V11

4ed830e

core: PATCH V12

66a0944

core: PATCH V13

426aaba

1. revert lock-free stat/timer 2. use drainExecuting 3. review comments

core: PATCH V14

1ed2ae7

1. go fmt

adtac force-pushed the adtac/profiling-core branch from 997e023 to 1ed2ae7 Compare December 5, 2019 21:54

core: PATCH V15: test fixes

682aa50

menghanl assigned menghanl and dfawley and unassigned adtac Dec 5, 2019

menghanl suggested changes Dec 6, 2019

View reviewed changes

internal/profiling/buffer/buffer.go Show resolved Hide resolved

internal/profiling/buffer/buffer.go Show resolved Hide resolved

internal/profiling/buffer/buffer.go Show resolved Hide resolved

internal/profiling/buffer/buffer.go Outdated Show resolved Hide resolved

menghanl suggested changes Dec 6, 2019

View reviewed changes

Adhityaa Chandrasekar added 2 commits December 6, 2019 15:44

core: PATCH V16

4bafcaa

core: PATCH V17

bbfb9f0

adtac force-pushed the adtac/profiling-core branch from 99d9d9b to bbfb9f0 Compare December 6, 2019 23:45

dfawley reviewed Dec 7, 2019

View reviewed changes

internal/profiling/buffer/buffer_appengine.go Outdated Show resolved Hide resolved

internal/profiling/profiling.go Show resolved Hide resolved

core: PATCH V18

ce2713c

dfawley approved these changes Dec 10, 2019

View reviewed changes

core: PATCH V19: CircularBuffer docs

69abad4

menghanl approved these changes Dec 10, 2019

View reviewed changes

dfawley changed the title ~~profiling: add internal changes~~ profiling: add internal changes to support profiling of gRPC Dec 11, 2019

dfawley merged commit 021bd57 into grpc:master Dec 11, 2019

dfawley mentioned this pull request Dec 11, 2019

profiling: add hooks within grpc #3159

Merged

github-actions bot locked as resolved and limited conversation to collaborators May 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

profiling: add internal changes to support profiling of gRPC #3158

profiling: add internal changes to support profiling of gRPC #3158

adtac commented Nov 7, 2019

dfawley commented Nov 11, 2019

adtac commented Nov 14, 2019

dfawley commented Nov 20, 2019

adtac commented Nov 20, 2019

dfawley left a comment

adtac commented Nov 20, 2019

adtac commented Nov 22, 2019

dfawley commented Nov 22, 2019

adtac commented Nov 22, 2019

dfawley left a comment

adtac commented Dec 5, 2019

adtac commented Dec 10, 2019

profiling: add internal changes to support profiling of gRPC #3158

profiling: add internal changes to support profiling of gRPC #3158

Conversation

adtac commented Nov 7, 2019

dfawley commented Nov 11, 2019

adtac commented Nov 14, 2019

dfawley commented Nov 20, 2019

adtac commented Nov 20, 2019

dfawley left a comment

Choose a reason for hiding this comment

adtac commented Nov 20, 2019

adtac commented Nov 22, 2019

dfawley commented Nov 22, 2019

adtac commented Nov 22, 2019

dfawley left a comment

Choose a reason for hiding this comment

adtac commented Dec 5, 2019

adtac commented Dec 10, 2019