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.

Sign up for GitHub

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

Jump to bottom

perf_bench_test start #54

Merged
merged 14 commits into from
Nov 20, 2020
Merged

perf_bench_test start #54

merged 14 commits into from
Nov 20, 2020

Conversation

NiuJ1ao
Copy link
Contributor

@NiuJ1ao NiuJ1ao commented Nov 11, 2020

Requests are injected every second and instance are served sequentially.

@NiuJ1ao NiuJ1ao requested a review from ustiugov November 11, 2020 15:36
@NiuJ1ao NiuJ1ao self-assigned this Nov 11, 2020
ustiugov
ustiugov requested changes Nov 11, 2020
View reviewed changes
Copy link
Member

@ustiugov ustiugov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good start and good coding style!

  1. Pulling images and creating snapshots, creating records must be outside of the measurement loop. In fact, the task was to profile warm invocations, not cold. Hence, snapshots, records are unnecessary at this point (but you can keep them for future).
  2. Place remove instance and flush caches calls under a runtime argument profile_cold_starts that you need to add

@NiuJ1ao NiuJ1ao removed their assignment Nov 15, 2020
@NiuJ1ao NiuJ1ao requested a review from ustiugov November 15, 2020 11:45
ustiugov
ustiugov requested changes Nov 15, 2020
View reviewed changes
Copy link
Member

@ustiugov ustiugov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to test by validating the logs. Currently, you confuse images and functions. These are different concepts.

The structure of the benchmark needs correction:

  1. Pull all necessary images by invoking funcPool.Serve() once per image
  2. Boot all functions with a single image (for the first version)
  3. Start the measurement loop

@NiuJ1ao NiuJ1ao requested a review from ustiugov November 17, 2020 12:12
ustiugov
ustiugov requested changes Nov 17, 2020
View reviewed changes
Copy link
Member

@ustiugov ustiugov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks much better now, thanks.

  1. Why do you need the semaphore?
  2. your benchmark sends a potentially large burst once per second. This may result in a high load period followed by a low load period each second. You should modify the code so that your benchmark injects 1 RPC per timeInterval=second/TargetReqPerSec

@NiuJ1ao NiuJ1ao requested a review from ustiugov November 17, 2020 17:57
ustiugov
ustiugov requested changes Nov 17, 2020
View reviewed changes
perf_bench_test.go Outdated Show resolved Hide resolved
perf_bench_test.go Outdated Show resolved Hide resolved
perf_bench_test.go Outdated Show resolved Hide resolved
perf_bench_test.go Outdated Show resolved Hide resolved
perf_bench_test.go Outdated Show resolved Hide resolved
perf_bench_test.go Outdated Show resolved Hide resolved
perf_bench_test.go Outdated Show resolved Hide resolved
@NiuJ1ao NiuJ1ao requested a review from ustiugov November 18, 2020 13:23
ustiugov
ustiugov requested changes Nov 18, 2020
View reviewed changes
Copy link
Member

@ustiugov ustiugov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor corrections are needed

perf_bench_test.go Outdated Show resolved Hide resolved
perf_bench_test.go Outdated Show resolved Hide resolved
perf_bench_test.go Outdated Show resolved Hide resolved
perf_bench_test.go Outdated Show resolved Hide resolved
perf_bench_test.go Outdated Show resolved Hide resolved
perf_bench_test.go Outdated Show resolved Hide resolved
perf_bench_test.go Show resolved Hide resolved
@NiuJ1ao NiuJ1ao requested a review from ustiugov November 19, 2020 12:45
ustiugov
ustiugov requested changes Nov 19, 2020
View reviewed changes
Copy link
Member

@ustiugov ustiugov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While there are few minor comments (mostly cleaning), you need to add a test for your benchmark, basically here

For example, you can invoke your benchmark to run load over 2 VMs for 2 seconds

perf_bench_test.go Outdated Show resolved Hide resolved
perf_bench_test.go Outdated Show resolved Hide resolved
perf_bench_test.go Outdated Show resolved Hide resolved
perf_bench_test.go Outdated Show resolved Hide resolved
perf_bench_test.go Outdated Show resolved Hide resolved
@NiuJ1ao NiuJ1ao requested a review from ustiugov November 20, 2020 11:05
ustiugov
ustiugov requested changes Nov 20, 2020
View reviewed changes
Copy link
Member

@ustiugov ustiugov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, hopefully last changes. the code looks very good now, good job

perf_bench_test.go Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
perf_bench_test.go Outdated Show resolved Hide resolved
@NiuJ1ao NiuJ1ao requested a review from ustiugov November 20, 2020 15:38
ustiugov
ustiugov previously approved these changes Nov 20, 2020
View reviewed changes
@NiuJ1ao NiuJ1ao requested a review from ustiugov November 20, 2020 17:59
@ustiugov ustiugov merged commit 75a4486 into master Nov 20, 2020
@ustiugov ustiugov deleted the perf_bench_test branch November 20, 2020 18:12
HermioneKT pushed a commit that referenced this pull request Jan 31, 2024
@KTCyber @HermioneKT
# This is a combination of 154 commits.
6674807 
# This is the 1st commit message:

refactor

# This is the commit message #2:

modify powerNodeSelector

# This is the commit message #3:

modify powerNodeSelector

# This is the commit message #4:

modify powerNodeSelector

# This is the commit message #5:

modify namespace

# This is the commit message #6:

modify namespace

# This is the commit message #7:

add namespace

# This is the commit message #8:

init power manager script

# This is the commit message #9:

add go code

# This is the commit message #10:

test

# This is the commit message #11:

test

# This is the commit message #12:

test

# This is the commit message #13:

test

# This is the commit message #14:

go.sum

# This is the commit message #15:

go sum

# This is the commit message #16:

go mod tidy

# This is the commit message #17:

test

# This is the commit message #18:

test

# This is the commit message #19:

test

# This is the commit message #20:

test

# This is the commit message #21:

test

# This is the commit message #22:

test

# This is the commit message #23:

test

# This is the commit message #24:

test

# This is the commit message #25:

test

# This is the commit message #26:

test

# This is the commit message #27:

test

# This is the commit message #28:

test

# This is the commit message #29:

print output

# This is the commit message #30:

test

# This is the commit message #31:

test

# This is the commit message #32:

test

# This is the commit message #33:

test

# This is the commit message #34:

test

# This is the commit message #35:

test

# This is the commit message #36:

test

# This is the commit message #37:

test

# This is the commit message #38:

test

# This is the commit message #39:

test

# This is the commit message #40:

refactor

# This is the commit message #41:

change val

# This is the commit message #42:

test

# This is the commit message #43:

test

# This is the commit message #44:

test

# This is the commit message #45:

exp1

# This is the commit message #46:

add url

# This is the commit message #47:

test

# This is the commit message #48:

test

# This is the commit message #49:

test auth

# This is the commit message #50:

test

# This is the commit message #51:

test

# This is the commit message #52:

test

# This is the commit message #53:

test

# This is the commit message #54:

test

# This is the commit message #55:

fix bug

# This is the commit message #56:

remove log

# This is the commit message #57:

test

# This is the commit message #58:

test

# This is the commit message #59:

test

# This is the commit message #60:

fix loop

# This is the commit message #61:

increase invocation

# This is the commit message #62:

test

# This is the commit message #63:

test

# This is the commit message #64:

test

# This is the commit message #65:

test

# This is the commit message #66:

test

# This is the commit message #67:

test

# This is the commit message #68:

test

# This is the commit message #69:

Test

# This is the commit message #70:

test

# This is the commit message #71:

test

# This is the commit message #72:

test

# This is the commit message #73:

test

# This is the commit message #74:

test

# This is the commit message #75:

test

# This is the commit message #76:

test

# This is the commit message #77:

test

# This is the commit message #78:

test

# This is the commit message #79:

test

# This is the commit message #80:

test

# This is the commit message #81:

test

# This is the commit message #82:

test

# This is the commit message #83:

test

# This is the commit message #84:

test

# This is the commit message #85:

test

# This is the commit message #86:

test

# This is the commit message #87:

test

# This is the commit message #88:

test

# This is the commit message #89:

test

# This is the commit message #90:

test1

# This is the commit message #91:

test

# This is the commit message #92:

test

# This is the commit message #93:

test

# This is the commit message #94:

test

# This is the commit message #95:

test

# This is the commit message #96:

test

# This is the commit message #97:

test

# This is the commit message #98:

test

# This is the commit message #99:

test

# This is the commit message #100:

test

# This is the commit message #101:

test

# This is the commit message #102:

test

# This is the commit message #103:

test

# This is the commit message #104:

test

# This is the commit message #105:

test

# This is the commit message #106:

test

# This is the commit message #107:

test

# This is the commit message #108:

test

# This is the commit message #109:

test1

# This is the commit message #110:

test

# This is the commit message #111:

test

# This is the commit message #112:

Test

# This is the commit message #113:

test

# This is the commit message #114:

test

# This is the commit message #115:

test

# This is the commit message #116:

test

# This is the commit message #117:

test

# This is the commit message #118:

test

# This is the commit message #119:

Test

# This is the commit message #120:

Test

# This is the commit message #121:

test

# This is the commit message #122:

test

# This is the commit message #123:

Test

# This is the commit message #124:

test

# This is the commit message #125:

test

# This is the commit message #126:

test

# This is the commit message #127:

test

# This is the commit message #128:

test

# This is the commit message #129:

test

# This is the commit message #130:

test

# This is the commit message #131:

test

# This is the commit message #132:

test

# This is the commit message #133:

test

# This is the commit message #134:

test

# This is the commit message #135:

test

# This is the commit message #136:

test

# This is the commit message #137:

test

# This is the commit message #138:

test

# This is the commit message #139:

test

# This is the commit message #140:

test

# This is the commit message #141:

test

# This is the commit message #142:

test

# This is the commit message #143:

Test

# This is the commit message #144:

Test

# This is the commit message #145:

Test

# This is the commit message #146:

Test

# This is the commit message #147:

test

# This is the commit message #148:

test

# This is the commit message #149:

test

# This is the commit message #150:

test

# This is the commit message #151:

test

# This is the commit message #152:

test

# This is the commit message #153:

test

# This is the commit message #154:

Test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ustiugov ustiugov ustiugov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@NiuJ1ao @ustiugov