Add benchmark for context handler#7047
Conversation
knative-prow-robot
added
the
size/M
knative-prow-robot
left a comment
knative-prow-robot
left a comment
There was a problem hiding this comment.
@MIBc: 0 warnings.
Details
In response to this:
/lint
Fixes: Part of ##6749
Proposed Changes
- Add benchmark for context handler.
Release Note
NoneBenchmark Result
goos: linux goarch: amd64 pkg: knative.dev/serving/pkg/activator/handler BenchmarkContextHandler/context_handler_success-sequential-4 316872 3824 ns/op 2072 B/op 19 allocs/op BenchmarkContextHandler/context_handler_success-parallel-4 407049 2902 ns/op 2072 B/op 19 allocs/op BenchmarkContextHandler/context_handler_failure-sequential-4 72049 16485 ns/op 5463 B/op 46 allocs/op BenchmarkContextHandler/context_handler_failure-parallel-4 62166 18198 ns/op 5535 B/op 46 allocs/op
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 kubernetes/test-infra repository.
|
Hi @MIBc. Thanks for your PR. I'm waiting for a knative member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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 kubernetes/test-infra repository. |
knative-prow-robot
added
needs-ok-to-test
|
/assign @markusthoemmes |
markusthoemmes
left a comment
markusthoemmes
left a comment
There was a problem hiding this comment.
/ok-to-test
Thanks!
| baseHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {}) | ||
|
|
||
| handler := NewContextHandler(ctx, baseHandler) | ||
| req := httptest.NewRequest(http.MethodGet, "http://example.com", nil).WithContext(ctx) |
There was a problem hiding this comment.
I don't think we should attach the context here. It's not needed AFAIK.
| } | ||
| ctx, cancel, _ := rtesting.SetupFakeContextWithCancel(&testing.T{}) | ||
| defer cancel() | ||
| revID := types.NamespacedName{Namespace: testNamespace, Name: testRevName} |
There was a problem hiding this comment.
This is redundant. You can just use testNamespace and testRevName when creating the revision below.
| }{ | ||
| { | ||
| label: "context handler success", | ||
| revisionName: testRevName, | ||
| }, | ||
| { | ||
| label: "context handler failure", | ||
| revisionName: "fake", | ||
| }, | ||
| } |
There was a problem hiding this comment.
| }{ | |
| { | |
| label: "context handler success", | |
| revisionName: testRevName, | |
| }, | |
| { | |
| label: "context handler failure", | |
| revisionName: "fake", | |
| }, | |
| } | |
| }{{ | |
| label: "context handler success", | |
| revisionName: testRevName, | |
| }, { | |
| label: "context handler failure", | |
| revisionName: "fake", | |
| }} |
You can collapse slice notations like this to spare some indentation and to improve readability a bit.
knative-prow-robot
added
ok-to-test
* Add benchmark for context handler.
MIBc
force-pushed
the
benchmark-activator
branch
from
4835e66 to
3d4a8c2
Compare
|
The following jobs failed:
Failed non-flaky tests preventing automatic retry of pull-knative-serving-autotls-tests: |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: markusthoemmes, MIBc The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow-robot
added
the
approved
|
/retest |
5 participants
/lint
Fixes: Part of ##6749
Proposed Changes
Release Note
None
Benchmark Result