You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by selvaganesang <gi...@git.apache.org> on 2016/04/30 01:55:21 UTC

[GitHub] incubator-trafodion pull request: [TRAFODION-1955] JNI optimizatio...

GitHub user selvaganesang opened a pull request:

    https://github.com/apache/incubator-trafodion/pull/459

    [TRAFODION-1955] JNI optimization at the time of compilation

    Removed ByteArrayList class and used array of byte array instead.
    Only one JNI to java transistion is done to getRegionStartKeys
    and getRegionEndKeys at the time of query compilation
    
    Also fixed an issue with lib_mgmt Makefile.

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

    $ git pull https://github.com/selvaganesang/incubator-trafodion trafodion-1955

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

    https://github.com/apache/incubator-trafodion/pull/459.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 #459
    
----
commit 6ef6f7c70eeaac17cbc9b0f22c0ffa3d0bf509b0
Author: selvaganesang <se...@esgyn.com>
Date:   2016-04-29T23:50:35Z

    [TRAFODION-1955] JNI optimization at the time of compilation
    
    Removed ByteArrayList class and used array of byte array instead.
    Only one JNI to java transistion is done to getRegionStartKeys
    and getRegionEndKeys at the time of query compilation
    
    Also fixed an issue with lib_mgmt Makefile.

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [TRAFODION-1955] JNI optimizatio...

Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/459#discussion_r61659592
  
    --- Diff: core/sql/executor/HBaseClient_JNI.cpp ---
    @@ -1439,14 +1223,14 @@ ByteArrayList* HBaseClient_JNI::getRegionStats(const char* tblName)
       jstring js_tblName = jenv_->NewStringUTF(tblName);
       if (js_tblName == NULL) 
       {
    -    GetCliGlobals()->setJniErrorStr(getErrorText(HBC_ERROR_DROP_PARAM));
    +    GetCliGlobals()->setJniErrorStr(getErrorText(HBC_ERROR_REGION_STATS));
    --- End diff --
    
    Good catch


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [TRAFODION-1955] JNI optimizatio...

Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/459#discussion_r61659862
  
    --- Diff: core/sql/executor/HBaseClient_JNI.cpp ---
    @@ -5619,3 +5394,41 @@ int convertStringObjectArrayToList(NAHeap *heap, jarray j_objArray,
         return arrayLen;
     }
     
    +
    +jint convertByteArrayObjectArrayToNAArray(NAHeap *heap, jarray j_objArray, NAArray<HbaseStr> **retArray)
    +{
    --- End diff --
    
    For safety, might be a good idea to set retArray = 0 first thing in the method. Otherwise a caller might ignore an error and use the uninitialized pointer, with unpredictable results.


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-trafodion pull request: [TRAFODION-1955] JNI optimizatio...

Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:

    https://github.com/apache/incubator-trafodion/pull/459#discussion_r61659355
  
    --- Diff: core/sql/executor/ExExeUtilGet.cpp ---
    @@ -3074,18 +3074,21 @@ short ExExeUtilGetHbaseObjectsTcb::work()
     
             case PROCESS_NEXT_ROW_:
               {
    -            if (currIndex_ == bal_->getSize())
    +            if (currIndex_ == hbaseTables_->entries())
                   {
                     step_ = DONE_;
                     break;
                   }
     
    -            Int32 len = 0;
    -            hbaseName_ = bal_->getEntry(currIndex_, hbaseNameBuf_, 
    -                                        ComMAX_3_PART_EXTERNAL_UTF8_NAME_LEN_IN_BYTES+6, 
    -                                        len);
    -            hbaseName_[len] = 0;
    -            
    +            Int32 len = hbaseTables_->at(currIndex_).len;
    +            char *tmp  = hbaseTables_->at(currIndex_).val;
    +            len = hbaseTables_->at(currIndex_).len;
    --- End diff --
    
    I hope the C++ compiler does common subexpression elimination so we don't keep re-evaluating the "at".


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---