Skip to content

perf(reporting): stop round-tripping the whole behaviour log through json for Elasticsearch - #3232

Open
doomedraven wants to merge 1 commit into
masterfrom
perf-reporting-elastic
Open

doomedraven wants to merge 1 commit into
masterfrom
perf-reporting-elastic

Conversation

@doomedraven

Copy link
Copy Markdown
Collaborator

The main one

report = json.loads(json.dumps(report, default=str), object_hook=self.date_hook)
new_processes = insert_calls(report, elastic_db=elastic_handler)

The round-trip serialises and re-parses the entire api call log — the largest structure in the report — one line before insert_calls moves it out into the calls index and discards it.

200k calls time
current 7358 ms
date_hook guard only 1760 ms
round-trip moved after insert_calls 2.1 ms
both 1.8 ms

date_hook

for key, value in json_dict.items():
    with suppress(Exception):
        json_dict[key] = datetime.strptime(value, "%Y-%m-%d %H:%M:%S")

Runs for every dict the parser produces and attempts strptime on every value, including ints and lists. Exception construction dominates. A leading digit, a - and a : are all necessary for that format to parse, so guarding on them converts exactly the same set of values — checked against the unguarded version across zero-padded and unpadded dates, boundary years, partial dates, T-separated timestamps, Windows paths, hex strings and empty strings.

Note

date_hook is not redundant with format_dates(). format_dates touches only info.started, info.ended, info.machine.{started_on,shutdown_on} and dropped[].pe.timestamp; the hook converts date-shaped strings anywhere in the tree. It is kept.

Warning

Moving the round-trip is a behaviour change for the calls index, and reviewers should weigh it:

  1. Call arguments no longer pass through default=str. They come from the BSON parser, so they are str/int/float/bool/None, but the safety net is gone.
  2. Call argument values shaped like %Y-%m-%d %H:%M:%S are no longer coerced to datetime before indexing, so they will map as strings rather than dates in the daily calls index. Arguably the correct behaviour, but it is mapping drift on an existing index.

If that is not acceptable, the date_hook guard alone still buys 4.2x and is behaviour-identical — happy to drop the reorder commit.

jsondump

store_compressed built the whole archive in a BytesIO via create_zip and then copied it out with getvalue(), on top of the uncompressed results dict already resident. Written straight to disk now. No measurable speed change — compression dominates either way — the point is peak memory. The archive entry name (reports/report.json) is preserved and asserted in a test.

Tests

tests/test_reporting_elastic_jsondump.py — 5 tests, no Elasticsearch instance needed. Equivalence of the guarded date_hook against the old one over 16 inputs, its behaviour as a real object_hook, and both jsondump paths including the archive entry name.

…json for Elasticsearch

ElasticSearchDB.run() did

    report = json.loads(json.dumps(report, default=str), object_hook=self.date_hook)
    new_processes = insert_calls(report, elastic_db=elastic_handler)

so the entire api call log - the largest structure in the report,
commonly millions of entries - was serialised and re-parsed one line
before insert_calls moved it out into the calls index. Moving the
round-trip after insert_calls takes a 200k-call report from 7.36s to
2ms.

date_hook attempted strptime on every value in the tree, constructing
an exception for each int, list and non-date string. A leading digit,
a '-' and a ':' are all necessary for the format to parse, so guarding
on them converts exactly the same set of values. Checked against the
unguarded version across padded and unpadded dates, boundary years,
partial dates, paths and empty strings. Worth a further 4.2x on the
work that remains.

date_hook is NOT redundant with format_dates, which only touches
info.started, info.ended, info.machine.* and dropped[].pe.timestamp.

jsondump with store_compressed built the whole archive in a BytesIO
and then copied it out with getvalue(). It writes the zip directly to
disk now. No measurable speed change; the point is peak memory. The
archive entry name is unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant