archive perf test #17522

dkijania · 2025-07-11T19:06:28Z

Enhanced precomputed_blocks test with performance metrics gathering. Addec CI job which also upload it to influx db and once we fill out 10 historical values every measurement will be checked for regression. Example test output:

[{"operation":"Zkapp_account_update.add","avg_time_ms":4.961491803278692},
{"operation":"Zkapp_account_update_body.add","avg_time_ms":3.0950926430517702},
{"operation":"Zkapp_actions.add","avg_time_ms":0.1407001117166212},
{"operation":"Zkapp_events.add","avg_time_ms":0.16317432697547685},
{"operation":"Zkapp_fee_payer_body.add","avg_time_ms":0.4071104508196722},
{"operation":"add_block","avg_time_ms":50.59486046511627},
{"operation":"adding_transactions","avg_time_ms":39.028174395348856},
{"operation":"block_and_zkapp_command.add_if_doesn't_exist","avg_time_ms":0.6587137950819674},
{"operation":"update_chain_status","avg_time_ms":0.3463889069767441},
{"operation":"zkapp_updates.add","avg_time_ms":0.4994985694822891}]

dkijania · 2025-07-11T19:16:16Z

!ci-build-me

dkijania · 2025-07-12T20:42:38Z

!ci-build-me

dkijania · 2025-07-14T18:43:48Z

!ci-build-me

dkijania · 2025-07-15T11:11:23Z

!ci-build-me

dkijania · 2025-07-15T11:27:10Z

!ci-build-me

dkijania · 2025-07-15T12:48:16Z

!ci-build-me

dkijania · 2025-07-16T07:44:39Z

!ci-build-me

dkijania · 2025-07-16T12:14:54Z

!ci-build-me

dkijania · 2025-07-16T16:11:30Z

!ci-build-me

dkijania · 2025-07-16T20:37:27Z

!ci-build-me

dkijania · 2025-07-16T22:09:01Z

!ci-build-me

glyh · 2025-07-17T02:04:23Z

scripts/benchmarks/lib/bench.py

@@ -444,6 +445,72 @@ def parse(self, content, output_filename, influxdb, branch):

        return [output_filename]

+class ArchiveBenchmark(Benchmark):
+    """
+     Concrete implementation of Benchmark for ledger test apply benchmark.


Confused, is this "ledger test apply" or "archive" benchmark?

Copy paste issue :(

Could you fix the name, then?

dkijania · 2025-07-17T07:25:59Z

!ci-build-me

glyh

Approving.

However, if this is one of the minority places we use InfluxDB in our CI, I'd consider removing InfluxDB as a whole and replace everything with Grafana, so to simplify our already complicated tech-stack

glyh · 2025-07-17T11:53:40Z

scripts/benchmarks/lib/bench.py

@@ -444,6 +445,72 @@ def parse(self, content, output_filename, influxdb, branch):

        return [output_filename]

+class ArchiveBenchmark(Benchmark):
+    """
+     Concrete implementation of Benchmark for ledger test apply benchmark.


Could you fix the name, then?

glyh · 2025-07-17T11:58:17Z

src/test/archive/archive_node_tests/archive_node_tests.ml

+  let%bind lines = Reader.file_lines log_file in
+  let perf_metrics =
+    List.filter_map lines ~f:(fun line ->
+        if String.is_substring line ~substring:" took " then


This is so fragile.

We have structured log, and parser specifically for structure logs, right?

Why can't we reuse that?

glyh

Approved too fast, I think it worth to factor out the manual log parsing part and use structured log parsing utilities we already have in our codebase.

glyh · 2025-07-18T00:06:06Z

src/test/archive/archive_node_tests/archive_node_tests.ml

+(* Convert performance metrics to a JSON format suitable for output *)
+(* The metrics are expected to be a list of tuples (operation, avg_time) *)
+(* where operation is a string and avg_time is a float representing the average time in milliseconds *)
+let perf_metrics_to_yojson metrics =


dkijania · 2025-07-18T18:59:44Z

!ci-build-me

glyh

Please consider remove the use of regex as I suggested

glyh · 2025-07-21T01:56:49Z

src/test/archive/archive_node_tests/archive_node_tests.ml

+              (* Extract the operation and time from the line *)
+              (* Parse the JSON line to extract the message field *)
+              let pattern =
+                Re.Perl.compile_pat {|(.+) took (\d+(?:\.\d+)?)(ms|us)|}


I think using regex is an anti-pattern when we could have store the log information in a structural way.

I'd consider replace these loggings to structural alternative

$ rg ' took ' lib/diff.ml 97: "Archive data generation for $state_hash: accounts-accessed took $time ms" 129: "Archive data generation for $state_hash: accounts-created took $time ms" lib/metrics.ml 10: "%s took %s" label

e.g.

in metrics.ml:

let time ~label f = let start = Time.now () in let%map x = f () in let stop = Time.now () in [%log' info (Logger.create ())] "%s took %s" label (Time.Span.to_string_hum (Time.diff stop start)) ; x

would be replaced by

let time ~label f = let start = Time.now () in let%map x = f () in let stop = Time.now () in let elapsed = Time.diff stop start in [%log' info (Logger.create ())] "%s took %s" label (Time.Span.to_string_hum elapsed) ~metadata: [ ("IS_ARCHIVE_PERF_METRICS", `Bool true) ; ("label", `String label) ; ("elapsed", `Float (Time.Span.to_ms elapsed)) ] ; x

Which is much easier to parse with yojson alone without regex.

dkijania · 2025-07-21T20:12:08Z

!ci-build-me

dkijania · 2025-07-21T20:51:11Z

!ci-build-me

glyh · 2025-07-22T05:18:55Z

buildkite/src/gen/Jobs.dhall

-- This file is autogenerated during builds. It remains checked in to ensure
-- dhall configuration can still execute locally without running codegen.
-let Pipeline = ../Pipeline/Dsl.dhall in [] : List Pipeline.CompoundType
+[ -- Autogenerated do not edit by hand ,


Are these still in the scope of this PR? I thought it's only for a test for archive nodes.

But anyway, I think it'll be infra engs reviewing this part.

glyh

Minor change requested, but LGTM overall

glyh · 2025-07-22T05:20:29Z

src/test/archive/archive_node_tests/archive_node_tests.ml

+(* Extract performance metrics from the log file *)
+(* where X is a floating point number representing the time taken for the operation *)
+(* log output should be in JSON format *)
+(* Example log line: {..., "message": "Operation took 123.45 ms"} *)


This comment is outdated.

glyh · 2025-07-22T05:22:57Z

src/app/archive/lib/diff.ml

-        [ ("state_hash", Mina_base.State_hash.to_yojson state_hash)
-        ; ( "time"
-          , `Float (Time.Span.to_ms (Time.diff accounts_accessed_time start)) )
-        ] ;


I assume this perf info is not important?

It is not needed as we are taking avg from all measurements.

glyh · 2025-07-22T05:28:56Z

src/test/archive/archive_node_tests/archive_node_tests.ml

+                  |> Option.value_exn
+                       ~message:
+                         ("Missing elapsed in log entry in log line: " ^ line)
+                  |> Yojson.Safe.to_string |> Float.of_string


It's better to use Yojson.Safe.Util.to_float to avoid an additional level of converison.

This reverts commit 00108a1.

dkijania · 2025-07-22T11:05:27Z

!ci-build-me

dkijania · 2025-07-22T14:27:54Z

!ci-build-me

dkijania · 2025-07-22T20:45:36Z

!ci-build-me

dkijania · 2025-07-22T21:25:42Z

!ci-build-me

dkijania force-pushed the dkijania/archive_perf_test_extract branch from a29563f to 00f0d43 Compare July 14, 2025 18:43

dkijania changed the title ~~Dkijania/archive perf test extract~~ archive perf test Jul 15, 2025

dkijania force-pushed the dkijania/archive_perf_test_extract branch from 7991eca to dd174be Compare July 15, 2025 11:22

dkijania self-assigned this Jul 15, 2025

dkijania marked this pull request as ready for review July 15, 2025 11:27

dkijania requested review from a team as code owners July 15, 2025 11:27

glyh reviewed Jul 17, 2025

View reviewed changes

glyh approved these changes Jul 18, 2025

View reviewed changes

glyh requested changes Jul 18, 2025

View reviewed changes

glyh requested changes Jul 21, 2025

View reviewed changes

glyh added tests performance labels Jul 21, 2025

dkijania force-pushed the dkijania/archive_perf_test_extract branch from c67d4ff to c76daee Compare July 21, 2025 20:48

dkijania added 2 commits July 21, 2025 22:48

add archive node tests to suite

88fe93d

eport performance result file

7e23516

dkijania added 15 commits July 21, 2025 22:48

enahnce benchmark app with new performance test

7ccd41b

add CI part to execute test and upload data

e2541d5

expand dirtyWhen filter

8164e6d

fixed duplicated debian components

fc0fd40

added changelog

2c8b59f

fix caching part

7015dc3

fix local path to archive.perf test

604e54b

remove extra args

5accd2e

set input file in extra args

9c06094

dhall lints

c514a36

fix benchmark name

c41f638

use structuted log insted of crude parsing

df947ca

use structured log instead of crude regex

1b1bc07

migrate to scopes

4a58c1f

fmt

00108a1

dkijania force-pushed the dkijania/archive_perf_test_extract branch from c069f60 to 00108a1 Compare July 21, 2025 20:50

glyh reviewed Jul 22, 2025

View reviewed changes

glyh approved these changes Jul 22, 2025

View reviewed changes

dkijania added 2 commits July 22, 2025 12:21

Revert "fmt"

365293d

This reverts commit 00108a1.

add docs and use Yojson.Safe.Util.to_float

833a2e8

remove extra args from arguments

62f73c6

remove trailing and leading quotas

b0abfc5

dkijania force-pushed the dkijania/archive_perf_test_extract branch from c7a7abd to b0abfc5 Compare July 22, 2025 21:25

archive perf test #17522

Are you sure you want to change the base?

archive perf test #17522

Uh oh!

Conversation

dkijania commented Jul 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dkijania commented Jul 11, 2025

Uh oh!

dkijania commented Jul 12, 2025

Uh oh!

dkijania commented Jul 14, 2025

Uh oh!

dkijania commented Jul 15, 2025

Uh oh!

dkijania commented Jul 15, 2025

Uh oh!

dkijania commented Jul 15, 2025

Uh oh!

dkijania commented Jul 16, 2025

Uh oh!

dkijania commented Jul 16, 2025

Uh oh!

dkijania commented Jul 16, 2025

Uh oh!

dkijania commented Jul 16, 2025

Uh oh!

dkijania commented Jul 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkijania commented Jul 17, 2025

Uh oh!

glyh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glyh left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkijania commented Jul 18, 2025

Uh oh!

glyh left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkijania commented Jul 21, 2025

Uh oh!

dkijania commented Jul 21, 2025

Uh oh!

glyh Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

glyh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dkijania commented Jul 11, 2025 •

edited

Loading

glyh left a comment •

edited

Loading

glyh left a comment •

edited

Loading

glyh Jul 22, 2025 •

edited

Loading