Improve performance of getSubstitutor by hit-lacus · Pull Request #1991 · apache/kylin

hit-lacus · 2022-09-29T02:27:04Z

Proposed changes

Describe the big picture of your changes here to communicate to the maintainers why we should accept this pull request. If it fixes a bug or resolves a feature request, be sure to link to that issue.

Branch to commit

Branch kylin3 for v2.x to v3.x
Branch kylin4 for v4.x
Branch kylin5 for v5.x

Types of changes

What types of changes does your code introduce to Kylin?
Put an x in the boxes that apply

Bugfix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation Update (if none of the other choices apply)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

I have created an issue on Kylin's jira, and have described the bug/feature there in detail
Commit messages in my PR start with the related jira ID, like "KYLIN-0000 Make Kylin project open-source"
Compiling and unit tests pass locally with my changes
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)
Any dependent changes have been merged

Further comments

If this is a relatively large or complex change, kick off the discussion at user@kylin.apache.org or dev@kylin.apache.org by explaining why you chose the solution you did and what alternatives you considered, etc...

hit-lacus · 2022-09-29T02:37:17Z

基于阳红的 patch，主要改动的是：

只有 KylinExternalConfigLoader 实现 ICachedExternalConfigLoader，NacosExternalConfigLoader 还是原来的接口
修改 size 的实现

zznlime · 2022-09-29T03:30:42Z

src/core-common/src/main/java/org/apache/kylin/common/PropertiesDelegate.java

+        } else if (configLoader instanceof KylinExternalConfigLoader) {
+            this.delegation = new CompositeMapView(((ICachedExternalConfigLoader)this.configLoader).getPropertyEntries(), this.properties);
+        } else if (configLoader instanceof NacosExternalConfigLoader) {
+            this.delegation = new CompositeMapView(((ICachedExternalConfigLoader)this.configLoader).getPropertyEntries(), this.properties);


Can this configloader cast into ICachedExternalConfigLoader if it is instanceof NacosExternalConfigLoader?

Sorry, that is my mistake, I will make it right.

hit-lacus · 2022-09-29T07:43:01Z

Load Test

Single query last for 10 minutes.

select count(*) from warehouse

Resource
1. 4 executors
2. 4G and 1 core for each executor
Load Testing
1. 40 concurrency
2. 5 concurrency

hit-lacus · 2022-09-29T10:09:17Z

Before Patch Applied

summary =  23880 in 00:08:09 =   48.8/s Avg:   817 Min:   254 Max: 14202 Err:     0 (0.00%)
summary +   1653 in 00:00:30 =   55.1/s Avg:   720 Min:   330 Max:  1297 Err:     0 (0.00%) Active: 40 Started: 40 Finished: 0
summary =  25533 in 00:08:39 =   49.2/s Avg:   810 Min:   254 Max: 14202 Err:     0 (0.00%)
summary +   1664 in 00:00:30 =   55.5/s Avg:   725 Min:   294 Max:  1324 Err:     0 (0.00%) Active: 40 Started: 40 Finished: 0
summary =  27197 in 00:09:09 =   49.5/s Avg:   805 Min:   254 Max: 14202 Err:     0 (0.00%)
summary +   1624 in 00:00:30 =   54.1/s Avg:   736 Min:   333 Max:  1336 Err:     0 (0.00%) Active: 40 Started: 40 Finished: 0
summary =  28821 in 00:09:39 =   49.8/s Avg:   801 Min:   254 Max: 14202 Err:     0 (0.00%)
summary +   1207 in 00:00:22 =   56.1/s Avg:   721 Min:   299 Max:  1391 Err:     0 (0.00%) Active: 0 Started: 40 Finished: 40
summary =  30028 in 00:10:01 =   50.0/s Avg:   798 Min:   254 Max: 14202 Err:     0 (0.00%)
Tidying up ...    @ Thu Sep 29 19:02:51 CST 2022 (1664449371507)
... end of run

hit-lacus · 2022-09-29T11:19:15Z

After patch applied

summary =  24970 in 00:08:07 =   51.2/s Avg:   778 Min:   255 Max: 11523 Err:     0 (0.00%)
summary +   1715 in 00:00:30 =   57.2/s Avg:   705 Min:   309 Max:  1281 Err:     0 (0.00%) Active: 40 Started: 40 Finished: 0
summary =  26685 in 00:08:37 =   51.6/s Avg:   773 Min:   255 Max: 11523 Err:     0 (0.00%)
summary +   1703 in 00:00:30 =   56.7/s Avg:   703 Min:   260 Max:  1256 Err:     0 (0.00%) Active: 40 Started: 40 Finished: 0
summary =  28388 in 00:09:07 =   51.9/s Avg:   769 Min:   255 Max: 11523 Err:     0 (0.00%)
summary +   1730 in 00:00:30 =   57.6/s Avg:   693 Min:   294 Max:  1225 Err:     0 (0.00%) Active: 40 Started: 40 Finished: 0
summary =  30118 in 00:09:37 =   52.2/s Avg:   764 Min:   255 Max: 11523 Err:     0 (0.00%)
summary +   1324 in 00:00:23 =   57.5/s Avg:   701 Min:   269 Max:  1388 Err:     0 (0.00%) Active: 0 Started: 40 Finished: 40
summary =  31442 in 00:10:00 =   52.4/s Avg:   762 Min:   255 Max: 11523 Err:     0 (0.00%)
Tidying up ...    @ Thu Sep 29 19:20:23 CST 2022 (1664450423085)
... end of run

hit-lacus · 2022-09-29T11:23:09Z

Performance Improve(40 threads)

(31442 - 30028)/30028 = 0.047  = 4.7%

hit-lacus · 2022-09-29T11:56:51Z

Performance Improve(5 threads)

After

summary =  17589 in 00:10:00 =   29.3/s Avg:   170 Min:   119 Max:  4837 Err:     0 (0.00%)

Before

summary =  17165 in 00:10:00 =   28.6/s Avg:   174 Min:   125 Max:  5736 Err:     0 (0.00%)

424/17165 = 0.024 = 2.4%

zznlime

LGTM

zznlime reviewed Sep 29, 2022

View reviewed changes

zznlime reviewed Sep 30, 2022

View reviewed changes

KYLIN-5274 Improve performance of getSubstitutor

40b0e5c

hit-lacus force-pushed the StrSubstitutor branch from d25e8fe to 40b0e5c Compare October 8, 2022 09:56

hit-lacus merged commit 88b5a43 into apache:kylin5 Oct 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve performance of getSubstitutor#1991

Improve performance of getSubstitutor#1991
hit-lacus merged 1 commit intoapache:kylin5from
hit-lacus:StrSubstitutor

hit-lacus commented Sep 29, 2022 •

edited

Loading

Uh oh!

hit-lacus commented Sep 29, 2022 •

edited

Loading

Uh oh!

zznlime Sep 29, 2022

Uh oh!

hit-lacus Sep 29, 2022

Uh oh!

hit-lacus commented Sep 29, 2022 •

edited

Loading

Uh oh!

hit-lacus commented Sep 29, 2022 •

edited

Loading

Uh oh!

hit-lacus commented Sep 29, 2022 •

edited

Loading

Uh oh!

hit-lacus commented Sep 29, 2022 •

edited

Loading

Uh oh!

hit-lacus commented Sep 29, 2022 •

edited

Loading

Uh oh!

zznlime left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

hit-lacus commented Sep 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Branch to commit

Types of changes

Checklist

Further comments

Uh oh!

hit-lacus commented Sep 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zznlime Sep 29, 2022

Choose a reason for hiding this comment

Uh oh!

hit-lacus Sep 29, 2022

Choose a reason for hiding this comment

Uh oh!

hit-lacus commented Sep 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Load Test

Uh oh!

hit-lacus commented Sep 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Before Patch Applied

Uh oh!

hit-lacus commented Sep 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

After patch applied

Uh oh!

hit-lacus commented Sep 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance Improve(40 threads)

Uh oh!

hit-lacus commented Sep 29, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance Improve(5 threads)

Uh oh!

zznlime left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hit-lacus commented Sep 29, 2022 •

edited

Loading

hit-lacus commented Sep 29, 2022 •

edited

Loading

hit-lacus commented Sep 29, 2022 •

edited

Loading

hit-lacus commented Sep 29, 2022 •

edited

Loading

hit-lacus commented Sep 29, 2022 •

edited

Loading

hit-lacus commented Sep 29, 2022 •

edited

Loading

hit-lacus commented Sep 29, 2022 •

edited

Loading