[GitHub] incubator-carbondata pull request #552: [CARBONDATA-661] misc cleanup in cor...

[qiuchenjian-2]

GitHub user JihongMA opened a pull request:

    https://github.com/apache/incubator-carbondata/pull/552

    [CARBONDATA-661] misc cleanup in core

    cleanup un-exercised code/field/functions as well as minor code improvement.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/JihongMA/incubator-carbondata carbondata-1

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/incubator-carbondata/pull/552.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #552
   
----
commit 1241ada35d3463eee2c9850590792b72bca4c416
Author: Jihong Ma <[hidden email]>
Date:   2016-12-13T00:31:12Z

    Merge remote-tracking branch 'upstream/master'

commit 0bf1fb35c79f0fe12cc0e61a8b6c53b51085bdcc
Author: Jihong Ma <[hidden email]>
Date:   2016-12-20T01:55:53Z

    Merge remote-tracking branch 'upstream/master'

commit f5483b6a58f4cdeec91850bf2127fba009eb6654
Author: Jihong Ma <[hidden email]>
Date:   2016-12-24T16:11:20Z

    Merge remote-tracking branch 'upstream/master'

commit 1600f4dc3314a492aedc8e6950fc9cbc33f0a76f
Author: Jihong Ma <[hidden email]>
Date:   2017-01-03T22:09:39Z

    Merge remote-tracking branch 'upstream/master'

commit 3bebe0a1f38258ebfe5ef5490fc1b95cb96e362f
Author: Jihong Ma <[hidden email]>
Date:   2017-01-05T18:41:39Z

    Merge remote-tracking branch 'upstream/master'

commit 5f991bb3ed05a185df4740feb4efa923a51e2345
Author: Jihong Ma <[hidden email]>
Date:   2017-01-10T02:21:49Z

    Merge remote-tracking branch 'upstream/master'

commit 5faad5d9e40dea0bd5d97bc346dbca903c17a496
Author: Jihong Ma <[hidden email]>
Date:   2017-01-10T18:58:33Z

    Merge remote-tracking branch 'upstream/master'

commit 0a00c54c1d6ceebb9ff15b8ea8a424a1e9523001
Author: Jihong Ma <[hidden email]>
Date:   2017-01-18T01:48:44Z

    rebase master

commit 3b9397d036a33f8e60f442bcd699bcacebb7a2d5
Author: Jihong Ma <[hidden email]>
Date:   2017-01-18T19:21:50Z

    misc code cleanup

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user CarbonDataQA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/552
 
    Build Success with Spark 1.6.2, Please check CI http://136.243.101.176:8080/job/ApacheCarbonPRBuilder/669/



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user jackylk commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/552
 
    Can you squash your commit into one?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/552#discussion_r96781406
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/keygenerator/columnar/impl/MultiDimKeyVarLengthEquiSplitGenerator.java ---
    @@ -143,17 +142,18 @@ private void intialize() {
     
       private int[] convertToArray(List<Integer> list) {
         int[] ints = new int[list.size()];
    -    for (int i = 0; i < ints.length; i++) {
    -      ints[i] = list.get(i);
    +    int i = 0;
    +    for (Integer ii : list) {
    --- End diff --
   
    foreach style for loop is not encouraged to use because of performance


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/552#discussion_r96781568
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/keygenerator/columnar/impl/MultiDimKeyVarLengthEquiSplitGenerator.java ---
    @@ -143,17 +142,18 @@ private void intialize() {
     
       private int[] convertToArray(List<Integer> list) {
         int[] ints = new int[list.size()];
    -    for (int i = 0; i < ints.length; i++) {
    -      ints[i] = list.get(i);
    +    int i = 0;
    +    for (Integer ii : list) {
    +      ints[i++] = ii.intValue();
         }
         return ints;
       }
     
       private int[] convertToArray(Set<Integer> set) {
         int[] ints = new int[set.size()];
         int i = 0;
    -    for (Iterator iterator = set.iterator(); iterator.hasNext(); ) {
    -      ints[i++] = (Integer) iterator.next();
    +    for (Integer ii: set) {
    --- End diff --
   
    foreach style for loop is not encouraged to use because of performance


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user jackylk commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/552#discussion_r96781582
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/keygenerator/columnar/impl/MultiDimKeyVarLengthEquiSplitGenerator.java ---
    @@ -143,17 +142,18 @@ private void intialize() {
     
       private int[] convertToArray(List<Integer> list) {
         int[] ints = new int[list.size()];
    -    for (int i = 0; i < ints.length; i++) {
    -      ints[i] = list.get(i);
    +    int i = 0;
    +    for (Integer ii : list) {
    +      ints[i++] = ii.intValue();
         }
         return ints;
       }
     
       private int[] convertToArray(Set<Integer> set) {
         int[] ints = new int[set.size()];
         int i = 0;
    -    for (Iterator iterator = set.iterator(); iterator.hasNext(); ) {
    -      ints[i++] = (Integer) iterator.next();
    +    for (Integer ii: set) {
    +      ints[i++]= ii.intValue();
    --- End diff --
   
    add space before `=`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user JihongMA commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/552#discussion_r96787067
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/keygenerator/columnar/impl/MultiDimKeyVarLengthEquiSplitGenerator.java ---
    @@ -143,17 +142,18 @@ private void intialize() {
     
       private int[] convertToArray(List<Integer> list) {
         int[] ints = new int[list.size()];
    -    for (int i = 0; i < ints.length; i++) {
    -      ints[i] = list.get(i);
    +    int i = 0;
    +    for (Integer ii : list) {
    --- End diff --
   
    the foreach is slower than get(i) an indexing search? not aware of it, and I thought the other way around


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user JihongMA commented on a diff in the pull request:

    https://github.com/apache/incubator-carbondata/pull/552#discussion_r96801631
 
    --- Diff: core/src/main/java/org/apache/carbondata/core/keygenerator/columnar/impl/MultiDimKeyVarLengthEquiSplitGenerator.java ---
    @@ -143,17 +142,18 @@ private void intialize() {
     
       private int[] convertToArray(List<Integer> list) {
         int[] ints = new int[list.size()];
    -    for (int i = 0; i < ints.length; i++) {
    -      ints[i] = list.get(i);
    +    int i = 0;
    +    for (Integer ii : list) {
    --- End diff --
   
    just checked online, the performance difference is on List.forEach() as well as stream()..


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user JihongMA closed the pull request at:

    https://github.com/apache/incubator-carbondata/pull/552


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

[qiuchenjian-2]

Github user JihongMA commented on the issue:

    https://github.com/apache/incubator-carbondata/pull/552
 
    close this PR for cleaner commit history.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---