Performance improvements for the event query API #7319

creachadair · 2021-11-25T04:02:37Z

Rework the implementation of event query parsing and execution to improve performance and reduce memory usage.

Previous memory and CPU profiles of the pubsub service showed query processing as a significant hotspot. While we don't have evidence that this is visibly hurting users, fixing it is fairly easy and self-contained.

Updates #6439.

Structure

Move the existing query implementation to the oldquery subdirectory. This can probably be deleted before merging.
Add a syntax subpackage providing a lexical scanner and parser to replace the generated PEG parser.
Pre-compile the query to avoid repeated syntax traversal and allocations during matching.
Update usage in the rest of the repository.

Benchmarks

Typical benchmark results comparing the original implementation (PEG) with the reworked implementation (Custom):

TEST                        TIME/OP  BYTES/OP  ALLOCS/OP  SPEEDUP   MEM SAVING
BenchmarkParsePEG-12       51716 ns  526832    27
BenchmarkParseCustom-12     2167 ns    4616    17         23.8x     99.1%
BenchmarkMatchPEG-12        3086 ns    1097    22
BenchmarkMatchCustom-12    294.2 ns      64     3         10.5x     94.1%

tac0turtle · 2021-11-25T11:02:50Z

closes #6439.

Amazing work!!

creachadair · 2021-11-25T15:46:54Z

closes #6439.

Amazing work!!

I'm not sure it entirely fixes #6439, quite a lot of memory per subscription is also spent on queuing and buffering messages for the clients. This should help some, though. 🙂

cmwaters

LGTM although I haven't really done a deep dive. I just have a few questions?

This doesn't change any query parsing behavior right?

libs/pubsub/query/query.go

libs/pubsub/query/bench_test.go

libs/pubsub/query/oldquery/query_test.go

libs/pubsub/query/syntax/parser.go

types/events_test.go

So we can still test and benchmark, but leaves the main package clear for the new implementation. Also: emove unused test case fields All the tests in this group are expected to compile. Remove the compile-error check field, which was always false. All the tests in this group do not want a match error. Remove the match-error check field.

These are the same test cases that the original implementation uses. Remove the one that doesn't pass for silly reasons, and document why.

Compiled -> Query update receiver names fix usage in test

Results: BenchmarkParsePEG-12 24410 48992 ns/op 526828 B/op 27 alloc/op BenchmarkParseCustom-12 566208 2150 ns/op 4616 B/op 17 alloc/op BenchmarkMatchPEG-12 396376 3082 ns/op 1097 B/op 22 alloc/op BenchmarkMatchCustom-12 4125183 287.4 ns/op 64 B/op 3 alloc/op

This ensures the examples are likely to be somewhat correct.

This replaces the old "Empty" query, whose name described the implementation rather than the effect.

Also update generated mocks. Directories: - internal/eventbus - internal/inspect - internal/state/indexer - libs/pubsub - types

Some of these are incredibly useless.

A follow-up to #7319.

A manual backport of #7319 and #7336.

A manual backport of #7319 and #7336. (cherry picked from commit 1c1ce83)

Rework the implementation of event query parsing and execution to improve performance and reduce memory usage. Previous memory and CPU profiles of the pubsub service showed query processing as a significant hotspot. While we don't have evidence that this is visibly hurting users, fixing it is fairly easy and self-contained. Updates tendermint#6439. Typical benchmark results comparing the original implementation (PEG) with the reworked implementation (Custom): ``` TEST TIME/OP BYTES/OP ALLOCS/OP SPEEDUP MEM SAVING BenchmarkParsePEG-12 51716 ns 526832 27 BenchmarkParseCustom-12 2167 ns 4616 17 23.8x 99.1% BenchmarkMatchPEG-12 3086 ns 1097 22 BenchmarkMatchCustom-12 294.2 ns 64 3 10.5x 94.1% ``` Components: * Add a basic parsing benchmark. * Move the original query implementation to a subdirectory. * Add lexical scanner for Query expressions. * Add a parser for Query expressions. * Implement query compiler. * Add test cases based on OpenAPI examples. * Add MustCompile to replace the original MustParse, and update usage.

…9334) * Performance improvements for the event query API (#7319) Rework the implementation of event query parsing and execution to improve performance and reduce memory usage. Previous memory and CPU profiles of the pubsub service showed query processing as a significant hotspot. While we don't have evidence that this is visibly hurting users, fixing it is fairly easy and self-contained. Updates #6439. Typical benchmark results comparing the original implementation (PEG) with the reworked implementation (Custom): ``` TEST TIME/OP BYTES/OP ALLOCS/OP SPEEDUP MEM SAVING BenchmarkParsePEG-12 51716 ns 526832 27 BenchmarkParseCustom-12 2167 ns 4616 17 23.8x 99.1% BenchmarkMatchPEG-12 3086 ns 1097 22 BenchmarkMatchCustom-12 294.2 ns 64 3 10.5x 94.1% ```

A follow-up to tendermint#7319.

…t#7319) (tendermint#9334) * Performance improvements for the event query API (tendermint#7319) Rework the implementation of event query parsing and execution to improve performance and reduce memory usage. Previous memory and CPU profiles of the pubsub service showed query processing as a significant hotspot. While we don't have evidence that this is visibly hurting users, fixing it is fairly easy and self-contained. Updates tendermint#6439. Typical benchmark results comparing the original implementation (PEG) with the reworked implementation (Custom): ``` TEST TIME/OP BYTES/OP ALLOCS/OP SPEEDUP MEM SAVING BenchmarkParsePEG-12 51716 ns 526832 27 BenchmarkParseCustom-12 2167 ns 4616 17 23.8x 99.1% BenchmarkMatchPEG-12 3086 ns 1097 22 BenchmarkMatchCustom-12 294.2 ns 64 3 10.5x 94.1% ```

creachadair force-pushed the mjf/weary-query branch from 0a03363 to 906ad70 Compare November 25, 2021 04:35

creachadair marked this pull request as ready for review November 25, 2021 04:47

creachadair requested review from cmwaters, ebuchman, tychoish and williambanfield as code owners November 25, 2021 04:47

creachadair force-pushed the mjf/weary-query branch 2 times, most recently from 96eb55d to 1d9e519 Compare November 28, 2021 01:50

cmwaters approved these changes Nov 29, 2021

View reviewed changes

libs/pubsub/query/query.go Show resolved Hide resolved

libs/pubsub/query/query.go Show resolved Hide resolved

tychoish reviewed Nov 29, 2021

View reviewed changes

M. J. Fromberger added 18 commits November 29, 2021 08:34

Add a basic parsing benchmark.

9d57f3d

Update go generate rules, synchronize with Makefile

5699d1f

Add lexical scanner for Query expressions.

b532794

Add a parser for Query expressions.

4e983e0

Add syntax package documentation.

dd7b631

parser: update grammar reference

7e92db6

syntax: add constants for time formats

999764f

Implement query compiler.

e34a420

Add query compiler tests.

98b455c

These are the same test cases that the original implementation uses. Remove the one that doesn't pass for silly reasons, and document why.

Replace original implementation.

aa53e78

Compiled -> Query update receiver names fix usage in test

Add test cases based on OpenAPI examples.

4da1357

This ensures the examples are likely to be somewhat correct.

Add an "All" query and tests for it.

1b75226

This replaces the old "Empty" query, whose name described the implementation rather than the effect.

Add MustCompile to replace the original MustParse.

20b26de

query: add Syntax method

427e882

Update query package usage

307eaad

Also update generated mocks. Directories: - internal/eventbus - internal/inspect - internal/state/indexer - libs/pubsub - types

Fix lint warnings.

1820335

Some of these are incredibly useless.

creachadair force-pushed the mjf/weary-query branch from f597fb0 to 9299438 Compare November 29, 2021 16:34

tychoish approved these changes Nov 29, 2021

View reviewed changes

Bump test timeout.

6368cd2

creachadair force-pushed the mjf/weary-query branch from 52f142e to 6368cd2 Compare November 29, 2021 19:57

creachadair merged commit 1dca1a8 into master Nov 29, 2021

creachadair deleted the mjf/weary-query branch November 29, 2021 21:08

creachadair pushed a commit that referenced this pull request Nov 29, 2021

Remove the PEG query implementation.

939f6f8

A follow-up to #7319.

This was referenced Nov 29, 2021

Remove the PEG query implementation. #7336

Merged

[pubsub] there is too highly memory usage when creating many events subscriptions #6439

Closed

creachadair pushed a commit that referenced this pull request Nov 29, 2021

Remove the PEG query implementation. (#7336)

99ee730

A follow-up to #7319.

creachadair pushed a commit that referenced this pull request Nov 29, 2021

Performance improvements for the event query API

41a6091

A manual backport of #7319 and #7336.

creachadair mentioned this pull request Nov 29, 2021

Performance improvements for the event query API #7338

Merged

creachadair pushed a commit that referenced this pull request Nov 29, 2021

Add pending change log entry for #7319.

0448d24

creachadair pushed a commit that referenced this pull request Nov 30, 2021

Add pending change log entry for #7319. (#7339)

c9f9095

creachadair pushed a commit that referenced this pull request Nov 30, 2021

Performance improvements for the event query API (#7338)

1c1ce83

A manual backport of #7319 and #7336.

creachadair mentioned this pull request Jan 24, 2022

ADR 074: RPC Event Subscription Interface #7677

Merged

This was referenced Jan 31, 2022

Redis-like subscriptions #874

Closed

Introduce queryCache to ease memory usage #1770

Closed

lklimek referenced this pull request in dashpay/tenderdash Mar 25, 2022

Performance improvements for the event query API (#7338)

5c89b8e

A manual backport of #7319 and #7336. (cherry picked from commit 1c1ce83)

yihuang mentioned this pull request Aug 23, 2022

rpc: port ADR-075 event subscription from master to main #9305

Closed

cmwaters mentioned this pull request Sep 8, 2022

backport: performance improvements for the event query API (#7319) #9334

Merged

3 tasks

mmsqe pushed a commit to mmsqe/tendermint that referenced this pull request Sep 22, 2022

Remove the PEG query implementation. (tendermint#7336)

53c427f

A follow-up to tendermint#7319.

jmalicevic mentioned this pull request Apr 27, 2023

pubsub: Properly parse big numbers cometbft/cometbft#769

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Performance improvements for the event query API #7319

Performance improvements for the event query API #7319

creachadair commented Nov 25, 2021 •

edited

tac0turtle commented Nov 25, 2021

creachadair commented Nov 25, 2021

cmwaters left a comment

Performance improvements for the event query API #7319

Performance improvements for the event query API #7319

Conversation

creachadair commented Nov 25, 2021 • edited

Structure

Benchmarks

tac0turtle commented Nov 25, 2021

creachadair commented Nov 25, 2021

cmwaters left a comment

Choose a reason for hiding this comment

creachadair commented Nov 25, 2021 •

edited