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

[CARBONDATA-861] Improvements in query #709

Closed
wants to merge 7 commits into from
Closed

[CARBONDATA-861] Improvements in query #709

wants to merge 7 commits into from

Conversation

ravipesala
Copy link
Contributor

Following are the list of improvements done in this part of PR.

  1. Removed multiple creation of array and copy of it in Dimension and measure chunk readers.
  2. Simplified logic of finding offsets of nodictionary keys in the class SafeVariableLengthDimensionDataChunkStore.
  3. Avoided byte array creation and copy for nodictionary columns in case of vectorized reader. Instead directly sending the length and offset to vector.
  4. Removed unnecessary decoder plan additions to oprtimized plan. It can optimize the codegen flow.
  5. Updated CompareTest to take table blocksize and kept as 32 Mb in order to make use of small sorting when doing take ordered in spark.

@asfbot
Copy link

asfbot commented Mar 29, 2017

Can one of the admins verify this patch?

2 similar comments
@asfbot
Copy link

asfbot commented Mar 29, 2017

Can one of the admins verify this patch?

@asfbot
Copy link

asfbot commented Mar 29, 2017

Can one of the admins verify this patch?

@CarbonDataQA
Copy link

Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/1380/

@ravipesala ravipesala force-pushed the minor-perf-improv branch 2 times, most recently from 12af0f8 to f8607a3 Compare March 30, 2017 08:15
@ravipesala ravipesala changed the title [WIP] Improvements in query [CARBONDATA-861] Improvements in query Apr 5, 2017
jackylk
jackylk reviewed Apr 5, 2017
View reviewed changes
examples/spark2/src/main/scala/org/apache/carbondata/examples/CompareTest.scala
@@ -306,6 +307,8 @@ object CompareTest {
// do GC and sleep for some time before running next table
System.gc()
Thread.sleep(1000)
System.gc()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this required?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no guarntee that GC will be called after calling of System.gc(), thats why after waiting for 1 second called again to increase the probability of running GC

jackylk
jackylk reviewed Apr 5, 2017
View reviewed changes
integration/spark2/src/main/scala/org/apache/spark/sql/CarbonDataFrameWriter.scala Outdated
} else {
""
}
)
if (property.nonEmpty && property.charAt(property.length-1) == ',') {
property = property.replace(property.length-1, property.length, "")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change property.length-1 to property.length - 1

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

@CarbonDataQA
Copy link

Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/1461/

@jackylk
Copy link
Contributor

jackylk commented Apr 11, 2017

@ravipesala please rebase

@CarbonDataQA
Copy link

Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/1560/

@jackylk
Copy link
Contributor

jackylk commented Apr 11, 2017

LGTM

@asfgit asfgit closed this in 4cdb7a2 Apr 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jackylk jackylk jackylk left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@ravipesala @asfbot @CarbonDataQA @jackylk