Details

    • Type: Sub-task
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.10.0
    • Component/s: Offheap, Storage
    • Labels:
      None

      Description

      We should release the buffer to the bufferPool instead of allocation and deallocation

        Issue Links

          Activity

          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user jinossy opened a pull request:

          https://github.com/apache/tajo/pull/224

          TAJO-1152: RawFile ByteBuffer should be reuse

          This patch depend on TAJO-1149.
          I've refactor the EOF checking

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

          $ git pull https://github.com/jinossy/tajo TAJO-1152

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

          https://github.com/apache/tajo/pull/224.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 #224


          commit aed406357e016e602b11dbaed3d20994dd651a17
          Author: jhkim <jhkim@apache.org>
          Date: 2014-10-31T12:10:08Z

          TAJO-1149: Implement direct read of TextFile scanner

          commit f95d8dc0ef04dfb82290e23d5430eefc70f2552d
          Author: jhkim <jhkim@apache.org>
          Date: 2014-10-31T13:42:47Z

          remove the past buffer reading

          commit f87e56f942b523f2bd2675d2c75484d3fea6023e
          Author: jhkim <jhkim@apache.org>
          Date: 2014-11-01T14:47:56Z

          refactor the compact buffer

          commit bd15ea07a0187ff1d76bb0ba887bf899a281aa07
          Author: jhkim <jhkim@apache.org>
          Date: 2014-11-03T08:03:30Z

          rename LineDelimitedReader to DelimitedLineReader

          commit 166e913be69d8ee7944d67c3fd82f015762d82b1
          Author: jhkim <jhkim@apache.org>
          Date: 2014-11-03T09:36:38Z

          rename csvfile.delimiter, csvfile.null to text.delimiter, text.null

          commit 48962d005cc39d2016b864e07d0f985eb31ed69f
          Author: jhkim <jhkim@apache.org>
          Date: 2014-11-03T14:22:01Z

          TAJO-1152: RawFile ByteBuffer should be reuse


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user jinossy opened a pull request: https://github.com/apache/tajo/pull/224 TAJO-1152 : RawFile ByteBuffer should be reuse This patch depend on TAJO-1149 . I've refactor the EOF checking You can merge this pull request into a Git repository by running: $ git pull https://github.com/jinossy/tajo TAJO-1152 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tajo/pull/224.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 #224 commit aed406357e016e602b11dbaed3d20994dd651a17 Author: jhkim <jhkim@apache.org> Date: 2014-10-31T12:10:08Z TAJO-1149 : Implement direct read of TextFile scanner commit f95d8dc0ef04dfb82290e23d5430eefc70f2552d Author: jhkim <jhkim@apache.org> Date: 2014-10-31T13:42:47Z remove the past buffer reading commit f87e56f942b523f2bd2675d2c75484d3fea6023e Author: jhkim <jhkim@apache.org> Date: 2014-11-01T14:47:56Z refactor the compact buffer commit bd15ea07a0187ff1d76bb0ba887bf899a281aa07 Author: jhkim <jhkim@apache.org> Date: 2014-11-03T08:03:30Z rename LineDelimitedReader to DelimitedLineReader commit 166e913be69d8ee7944d67c3fd82f015762d82b1 Author: jhkim <jhkim@apache.org> Date: 2014-11-03T09:36:38Z rename csvfile.delimiter, csvfile.null to text.delimiter, text.null commit 48962d005cc39d2016b864e07d0f985eb31ed69f Author: jhkim <jhkim@apache.org> Date: 2014-11-03T14:22:01Z TAJO-1152 : RawFile ByteBuffer should be reuse
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          https://github.com/apache/tajo/pull/224#discussion_r20142448

          — Diff: tajo-storage/src/main/java/org/apache/tajo/storage/RawFile.java —
          @@ -109,56 +106,50 @@ public void init() throws IOException {

          // initial read
          if (fragment.getStartKey() > 0)

          { - channel.position(fragment.getStartKey()); + channel.position(fragment.getStartKey()); }
          • numBytesRead = channel.read(buffer);
          • buffer.flip();

          + forceFillBuffer = true;
          super.init();
          }

          @Override
          public long getNextOffset() throws IOException

          { - return channel.position() - buffer.remaining(); + return filePosition - (forceFillBuffer ? 0 : buffer.remaining()); }

          @Override
          public void seek(long offset) throws IOException {

          • long currentPos = channel.position();
          • if(currentPos < offset && offset < currentPos + buffer.limit()){
          • buffer.position((int)(offset - currentPos));
            + eos = false;
            + filePosition = channel.position();
            +
              • End diff –

          Could you add the comment in this line?

          ```
          // do not fill the buffer if the offset is already included in the buffer.
          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on a diff in the pull request: https://github.com/apache/tajo/pull/224#discussion_r20142448 — Diff: tajo-storage/src/main/java/org/apache/tajo/storage/RawFile.java — @@ -109,56 +106,50 @@ public void init() throws IOException { // initial read if (fragment.getStartKey() > 0) { - channel.position(fragment.getStartKey()); + channel.position(fragment.getStartKey()); } numBytesRead = channel.read(buffer); buffer.flip(); + forceFillBuffer = true; super.init(); } @Override public long getNextOffset() throws IOException { - return channel.position() - buffer.remaining(); + return filePosition - (forceFillBuffer ? 0 : buffer.remaining()); } @Override public void seek(long offset) throws IOException { long currentPos = channel.position(); if(currentPos < offset && offset < currentPos + buffer.limit()){ buffer.position((int)(offset - currentPos)); + eos = false; + filePosition = channel.position(); + End diff – Could you add the comment in this line? ``` // do not fill the buffer if the offset is already included in the buffer. ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user hyunsik commented on the pull request:

          https://github.com/apache/tajo/pull/224#issuecomment-62530492

          +1

          The patch looks good to me. This work will be helpful to efficiently manage direct memory in JVM. Could you add one comment to the source before committing the patch?

          Show
          githubbot ASF GitHub Bot added a comment - Github user hyunsik commented on the pull request: https://github.com/apache/tajo/pull/224#issuecomment-62530492 +1 The patch looks good to me. This work will be helpful to efficiently manage direct memory in JVM. Could you add one comment to the source before committing the patch?
          Hide
          jhkim Jinho Kim added a comment -

          Committed it. Thank you for the review!

          Show
          jhkim Jinho Kim added a comment - Committed it. Thank you for the review!
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Tajo-master-build #446 (See https://builds.apache.org/job/Tajo-master-build/446/)
          TAJO-1152: RawFile ByteBuffer should be reuse. (jinho) (jhkim: rev 55084a8ae29a3650409c1f1dea68223d6f1c340d)

          • tajo-storage/src/main/java/org/apache/tajo/storage/RawFile.java
          • tajo-storage/src/test/java/org/apache/tajo/storage/TestStorages.java
          • CHANGES
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Tajo-master-build #446 (See https://builds.apache.org/job/Tajo-master-build/446/ ) TAJO-1152 : RawFile ByteBuffer should be reuse. (jinho) (jhkim: rev 55084a8ae29a3650409c1f1dea68223d6f1c340d) tajo-storage/src/main/java/org/apache/tajo/storage/RawFile.java tajo-storage/src/test/java/org/apache/tajo/storage/TestStorages.java CHANGES
          Hide
          hudson Hudson added a comment -

          FAILURE: Integrated in Tajo-master-CODEGEN-build #88 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/88/)
          TAJO-1152: RawFile ByteBuffer should be reuse. (jinho) (jhkim: rev 55084a8ae29a3650409c1f1dea68223d6f1c340d)

          • tajo-storage/src/test/java/org/apache/tajo/storage/TestStorages.java
          • tajo-storage/src/main/java/org/apache/tajo/storage/RawFile.java
          • CHANGES
          Show
          hudson Hudson added a comment - FAILURE: Integrated in Tajo-master-CODEGEN-build #88 (See https://builds.apache.org/job/Tajo-master-CODEGEN-build/88/ ) TAJO-1152 : RawFile ByteBuffer should be reuse. (jinho) (jhkim: rev 55084a8ae29a3650409c1f1dea68223d6f1c340d) tajo-storage/src/test/java/org/apache/tajo/storage/TestStorages.java tajo-storage/src/main/java/org/apache/tajo/storage/RawFile.java CHANGES
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/tajo/pull/224

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/tajo/pull/224

            People

            • Assignee:
              jhkim Jinho Kim
              Reporter:
              jhkim Jinho Kim
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development