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.

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

Functions that traverse the IProfilingBlockInputStream tree may have performance issues when the tree contains SharedQueryBlockInputStream #7810

Closed
windtalker opened this issue Jul 17, 2023 · 4 comments · Fixed by #7854
Labels
affects-5.0 affects-5.1 affects-5.2 affects-5.3 affects-5.4 This bug affects the 5.4.x(LTS) versions. affects-6.0 affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.2 affects-6.3 affects-6.4 affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-6.6 affects-7.0 affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.2 affects-7.3 component/compute severity/major type/bug The issue is confirmed as a bug.

Comments

@windtalker
Copy link
Contributor

windtalker commented Jul 17, 2023

Bug Report

The same issue as #4494
The root cause is when IProfilingBlockInputStream tree contains SharedQueryBlockInputStream, the tree is actually a DAG, and when traverse the DAG as a tree, it need much redundant traverse, at worst case, the time complexity become O(m^n), where n is the number of SharedQueryBlockInputStream in the DAG, m is the number of children for the tree node.
Currently, the functions that may have this problem inlcudes:

  • IProfilingBlockInputStream::setProcessListElement
  • IProfilingBlockInputStream::setProgressCallback
@windtalker windtalker changed the title Functions that traverse the IProfilingBlockInputStream tree may have performance issues where the tree contains SharedQueryBlockInputStream Functions that traverse the IProfilingBlockInputStream tree may have performance issues when the tree contains SharedQueryBlockInputStream Jul 17, 2023
@SeaRise
Copy link
Contributor

SeaRise commented Jul 17, 2023

It looks like collectCPUTimeNsImpl won't have this problem because cpu_time_ns_collected is used in collectCPUTimeNs to avoid calling collectCPUTimeNsImpl repeatedly

@windtalker
Copy link
Contributor Author

windtalker commented Jul 17, 2023

OK, I've updated the issue description.

@SeaRise SeaRise mentioned this issue Jul 27, 2023
12 tasks
@yibin87
Copy link
Contributor

yibin87 commented Jul 31, 2023

Can we fix the issue by override these interfaces in SharedQueryBlockInputStream, and check if redundant ops?

@SeaRise
Copy link
Contributor

SeaRise commented Jul 31, 2023

Can we fix the issue by override these interfaces in SharedQueryBlockInputStream, and check if redundant ops?

Because these two functions are useless in TiFlash, they can be omitted.

@ti-chi-bot ti-chi-bot bot closed this as completed in #7854 Aug 1, 2023
ti-chi-bot bot pushed a commit that referenced this issue Aug 1, 2023
ti-chi-bot bot pushed a commit that referenced this issue Aug 1, 2023
ti-chi-bot bot pushed a commit that referenced this issue Aug 9, 2023
ti-chi-bot bot pushed a commit that referenced this issue Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects-5.0 affects-5.1 affects-5.2 affects-5.3 affects-5.4 This bug affects the 5.4.x(LTS) versions. affects-6.0 affects-6.1 This bug affects the 6.1.x(LTS) versions. affects-6.2 affects-6.3 affects-6.4 affects-6.5 This bug affects the 6.5.x(LTS) versions. affects-6.6 affects-7.0 affects-7.1 This bug affects the 7.1.x(LTS) versions. affects-7.2 affects-7.3 component/compute severity/major type/bug The issue is confirmed as a bug.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants