Skip to content

chore(cts): report benchmark results #3406

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

Merged
merged 4 commits into from
Jul 22, 2024
Merged

chore(cts): report benchmark results #3406

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2066523
chore(cts): report benchmark results
millotp Jul 21, 2024
a48923a
pretty table
millotp Jul 21, 2024
59197cf
empty line
millotp Jul 22, 2024
64f7b3d
add disclaimer
millotp Jul 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions .github/workflows/check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,9 @@ jobs:
if: ${{ !github.event.pull_request.head.repo.fork && steps.cts-e2e.outcome == 'failure' }}
run: yarn cli cts run javascript ${{ fromJSON(needs.setup.outputs.JAVASCRIPT_DATA).toRun }} --no-client --no-requests

- name: Run benchmarks
run: yarn cli cts run javascript ${{ fromJSON(needs.setup.outputs.JAVASCRIPT_DATA).toRun }} --benchmark --no-client --no-requests --no-e2e

- name: Generate code snippets for documentation
run: yarn cli snippets javascript ${{ fromJSON(needs.setup.outputs.JAVASCRIPT_DATA).toRun }}

Expand Down Expand Up @@ -348,6 +351,9 @@ jobs:
if: ${{ !github.event.pull_request.head.repo.fork && steps.cts-e2e.outcome == 'failure' }}
run: yarn cli cts run ${{ matrix.client.language }} ${{ matrix.client.toRun }} --no-client --no-requests

- name: Run benchmarks
run: yarn cli cts run ${{ matrix.client.language }} ${{ matrix.client.toRun }} --benchmark --no-client --no-requests --no-e2e

- name: Generate code snippets for documentation
run: yarn cli snippets ${{ matrix.client.language }} ${{ matrix.client.toRun }}

Expand Down Expand Up @@ -474,6 +480,26 @@ jobs:
- name: Generate documentation specs with code snippets
run: yarn cli build specs ${{ fromJSON(needs.setup.outputs.SPECS_MATRIX).toRun }} --docs

- name: Read benchmark results
id: benchmark
run: |
# merge all benchmark results into a single json, and remove the files
results=$(jq -s 'add' -c tests/output/**/benchmarkResult.json)
{
echo 'BENCHMARK_SECTION<<EOF'
echo "<details>"
echo "<summary>📊 Benchmark results</summary>"
Copy link
Member

Choose a reason for hiding this comment

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

can we add a small disclaimer like

take those results with a grain of salt, best effort benchmarking solution, in order to highlight general performances of our Search API

or something like that? just so people don't expect those to be a source of truth or things like that

Copy link
Member

Choose a reason for hiding this comment

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

also we should mention that it's only tested on search?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

added a small disclaimer, I would like to add more benchmark in the future so I'm not very specific.

echo "" # empty line is required to make the table work
Copy link
Member

Choose a reason for hiding this comment

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

do we still need it now that we have a comment?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

need what ?

Copy link
Member

Choose a reason for hiding this comment

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

this specific line 492 which add an empty line

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Screenshot 2024-07-22 at 14 43 11

yes otherwise it looks like this

Copy link
Member

Choose a reason for hiding this comment

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

ahhh ok make sense

echo "Benchmarks performed on the `search` method using a mock server, the results might not reflect the real-world performance."
# format the json to a markdown table with column "Language" and "rate"
echo "| Language | Rate |"
echo "| :------- | ---: |"
echo "$results" | jq -r 'to_entries | map([.key, .value.rate]) | sort_by(.[1]) | reverse | .[] | @tsv' | awk -F'\t' '{printf "| %-10s | %10d |\n", $1, $2}'
echo "</details>"
echo 'EOF'
} >> "$GITHUB_OUTPUT"
rm -rf tests/output/**/benchmarkResult.json

- name: Push generated code
id: pushGeneratedCode
run: yarn workspace scripts pushGeneratedCode
Expand All @@ -491,6 +517,8 @@ jobs:

_If you believe code should've been generated, please, [report the issue](https://github.com/algolia/api-clients-automation/issues/new?assignees=&labels=bug%2Ctriage&projects=&template=Bug_report.yml&title=%5Bbug%5D%3A+)._

${{ steps.benchmark.outputs.BENCHMARK_SECTION }}

- name: update generation comment
uses: marocchino/sticky-pull-request-comment@v2
if: ${{ steps.pushGeneratedCode.outputs.GENERATED_COMMIT != '' }}
Expand All @@ -505,6 +533,8 @@ jobs:
| 🍃 Generated commit | [`${{ steps.pushGeneratedCode.outputs.GENERATED_COMMIT }}`](${{ github.event.pull_request.base.repo.html_url }}/commit/${{ steps.pushGeneratedCode.outputs.GENERATED_COMMIT }}) |
| 🌲 Generated branch | [`generated/${{ github.head_ref }}`](${{ github.event.pull_request.base.repo.html_url }}/tree/generated/${{ github.head_ref }}) |

${{ steps.benchmark.outputs.BENCHMARK_SECTION }}

- name: Build website
if: ${{ github.ref == 'refs/heads/main' || github.base_ref == 'main' }}
run: yarn website:build
Expand Down
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ pom.xml
dist
.cache

benchmarkResult.json

**.openapi-generator
/openapitools.json
**/.openapi-generator-ignore
Expand Down
1 change: 1 addition & 0 deletions generators/src/main/java/com/algolia/codegen/cts/tests/TestsGenerator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ private String languageCased() {
private String injectVariables(String json) {
long threeDays = 3 * 24 * 60 * 60 * 1000;
json = json
.replace("${{language}}", language)
.replace("${{languageCased}}", languageCased())
.replace("${{clientPascalCase}}", Helpers.capitalize(Helpers.camelize(client)))
.replace("\"${{nowRounded}}\"", String.valueOf(Math.round(System.currentTimeMillis() / threeDays) * threeDays));
Expand Down
2 changes: 1 addition & 1 deletion scripts/ci/githubActions/createMatrix.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ async function createClientMatrix(baseBranch: string): Promise<void> {
const clientName = createClientName(client, language);
const extension = getTestExtension(language);

return `${testsOutputBase}/client/${clientName}${extension} ${testsOutputBase}/requests/${clientName}${extension} ${testsOutputBase}/e2e/${clientName}${extension} ${testsOutputBase}/benchmark/${clientName}${extension}`;
return `${testsOutputBase}/client/${clientName}${extension} ${testsOutputBase}/requests/${clientName}${extension} ${testsOutputBase}/e2e/${clientName}${extension} ${testsOutputBase}/benchmark/${clientName}${extension} ${testsRootFolder}/benchmarkResult.json`;
})
.join(' ');

Expand Down
12 changes: 12 additions & 0 deletions scripts/cts/testServer/benchmark.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
/* eslint-disable no-console */
import fs from 'fs';
import type { Server } from 'http';

import { expect } from 'chai';
import chalk from 'chalk';
import express from 'express';
import type { Express } from 'express';

import { CI, toAbsolutePath } from '../../common';

import { setupServer } from '.';

const benchmarkStatus: Record<
Expand All @@ -31,6 +34,15 @@ export function printBenchmarkReport(): void {
// eslint-disable-next-line no-nested-ternary
rate > 2000 ? 'bgGreenBright' : rate > 1000 ? 'bgGreen' : rate > 500 ? 'bgYellow' : 'bgRed';
console.log(chalk.black[color](`${lang}: ${Math.floor(rate)} req/s`));

if (CI) {
// save the result to a file, to be reported in the CI.
// we can't use setOutput here, because it doesn't work with matrix strategies
fs.writeFileSync(
toAbsolutePath(`tests/output/${lang}/benchmarkResult.json`),
JSON.stringify({ [lang]: { rate } }),
);
}
}
}

Expand Down
6 changes: 3 additions & 3 deletions scripts/cts/testServer/timeout.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@ export function assertValidTimeouts(expectedCount: number): void {

// languages are not consistent yet for the delay between requests
switch (lang) {
case 'JavaScript':
case 'javascript':
expect(state.duration[0] * 4).to.be.closeTo(state.duration[1], 200);
break;
case 'PHP':
case 'php':
expect(state.duration[0] * 2).to.be.closeTo(state.duration[1], 200);
break;
case 'Swift':
case 'swift':
expect(state.duration[0]).to.be.closeTo(state.duration[1], 800);
break;
default:
Expand Down
2 changes: 1 addition & 1 deletion tests/CTS/benchmark/search/benchmark.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"parameters": {
"requests": [
{
"indexName": "cts_e2e_benchmark_search_${{languageCased}}",
"indexName": "cts_e2e_benchmark_search_${{language}}",
"query": "iphone 15 pro max 512gb",
"hitsPerPage": 50
}
Expand Down
2 changes: 1 addition & 1 deletion tests/CTS/client/search/api.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
"type": "method",
"method": "customGet",
"parameters": {
"path": "1/test/retry/${{languageCased}}"
"path": "1/test/retry/${{language}}"
},
"expected": {
"type": "response",
Expand Down
20 changes: 10 additions & 10 deletions tests/CTS/client/search/helpers.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
"type": "method",
"method": "replaceAllObjects",
"parameters": {
"indexName": "cts_e2e_replace_all_objects_${{languageCased}}",
"indexName": "cts_e2e_replace_all_objects_${{language}}",
"objects": [
{
"objectID": "1",
Expand Down Expand Up @@ -193,7 +193,7 @@
"type": "method",
"method": "saveObjects",
"parameters": {
"indexName": "cts_e2e_saveObjects_${{languageCased}}",
"indexName": "cts_e2e_saveObjects_${{language}}",
"objects": [
{
"objectID": "1",
Expand Down Expand Up @@ -241,7 +241,7 @@
"type": "method",
"method": "partialUpdateObjects",
"parameters": {
"indexName": "cts_e2e_partialUpdateObjects_${{languageCased}}",
"indexName": "cts_e2e_partialUpdateObjects_${{language}}",
"objects": [
{
"objectID": "1",
Expand Down Expand Up @@ -290,7 +290,7 @@
"type": "method",
"method": "partialUpdateObjects",
"parameters": {
"indexName": "cts_e2e_partialUpdateObjects_${{languageCased}}",
"indexName": "cts_e2e_partialUpdateObjects_${{language}}",
"objects": [
{
"objectID": "3",
Expand Down Expand Up @@ -339,7 +339,7 @@
"type": "method",
"method": "deleteObjects",
"parameters": {
"indexName": "cts_e2e_deleteObjects_${{languageCased}}",
"indexName": "cts_e2e_deleteObjects_${{language}}",
"objectIDs": [
"1",
"2"
Expand Down Expand Up @@ -381,13 +381,13 @@
"type": "method",
"method": "waitForApiKey",
"parameters": {
"key": "api-key-add-operation-test-${{languageCased}}",
"key": "api-key-add-operation-test-${{language}}",
"operation": "add"
},
"expected": {
"type": "response",
"match": {
"value": "api-key-add-operation-test-${{languageCased}}",
"value": "api-key-add-operation-test-${{language}}",
"description": "my new api key",
"acl": [
"search",
Expand Down Expand Up @@ -423,7 +423,7 @@
"type": "method",
"method": "waitForApiKey",
"parameters": {
"key": "api-key-update-operation-test-${{languageCased}}",
"key": "api-key-update-operation-test-${{language}}",
"operation": "update",
"apiKey": {
"description": "my updated api key",
Expand All @@ -448,7 +448,7 @@
"expected": {
"type": "response",
"match": {
"value": "api-key-update-operation-test-${{languageCased}}",
"value": "api-key-update-operation-test-${{language}}",
"description": "my updated api key",
"acl": [
"search",
Expand Down Expand Up @@ -493,7 +493,7 @@
"type": "method",
"method": "waitForApiKey",
"parameters": {
"key": "api-key-delete-operation-test-${{languageCased}}",
"key": "api-key-delete-operation-test-${{language}}",
"operation": "delete"
},
"expected": {
Expand Down
Loading