Skip to content
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

Jump to bottom

Investigate performance regression of json parser #10840

Closed
kwannoel opened this issue Jul 10, 2023 · 7 comments
Closed

Investigate performance regression of json parser #10840

kwannoel opened this issue Jul 10, 2023 · 7 comments
Assignees
@tabVersion @kwannoel
Labels
type/feature
Milestone
release-1.1

Comments

@kwannoel
Copy link
Contributor

kwannoel commented Jul 10, 2023 •
edited
Loading

Originally from slack:

Fyi seems like json micro-bench recently regress on 15th june 2023. Execution duration jumped from ~450M ns to ~600M ns.
Maybe can take a look at the PRs merged then. https:/risingwavelabs/risingwave/pulls?q=is%3Apr+merged%3A2023-06-14..2023-06-15+is%3Aclosed

image

@tabVersion I think it might be #10096
@github-actions github-actions bot added this to the release-0.20 milestone Jul 10, 2023
@kwannoel kwannoel changed the title Performance regression of json parser Investigate performance regression of json parser Jul 10, 2023
@BugenZhao
Copy link
Member

May worth checking #10096.

@lmatz
Copy link
Contributor

lmatz commented Jul 10, 2023

Suspect some unnecessary clone copy is introduced...

@lmatz
Copy link
Contributor

lmatz commented Jul 10, 2023

SCR-20230710-ny1
using flame graph from q0: https://buildkite.com/risingwavelabs/cpu-flamegraph-weekly-cron/builds/18#_

@lmatz
Copy link
Contributor

lmatz commented Jul 13, 2023

#10919

We also need to track other parser...

@lmatz lmatz changed the title Investigate performance regression of json parser Investigate performance regression of json parser and micro-benchmark all the parsers Jul 13, 2023
@kwannoel kwannoel modified the milestones: release-1.0, release-1.1 Jul 14, 2023
@kwannoel kwannoel self-assigned this Jul 17, 2023
This was referenced Jul 17, 2023
Merged
Open
@kwannoel
Copy link
Contributor Author

#10919

We also need to track other parser...

Good point. Opened a separate issue for it.

@kwannoel kwannoel changed the title Investigate performance regression of json parser and micro-benchmark all the parsers Investigate performance regression of json parser Jul 17, 2023
@kwannoel
Copy link
Contributor Author

Btw after #10971

It seems like another introduced cost is from catch_unwind.
The flamegraph does not provide sufficient information on the source of catch_unwind however. Not too sure how to identify the underlying cost.

Before:
Screenshot 2023-07-17 at 2 48 14 PM

Current cost:
Screenshot 2023-07-17 at 2 47 46 PM

@kwannoel
Copy link
Contributor Author

kwannoel commented Aug 1, 2023

q0 shows it's more or less back to normal.

@kwannoel kwannoel closed this as not planned Won't fix, can't repro, duplicate, stale Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tabVersion tabVersion

@kwannoel kwannoel

Labels
type/feature
Projects
None yet
Milestone
release-1.1
Development

No branches or pull requests
4 participants
@lmatz @tabVersion @BugenZhao @kwannoel