You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@orc.apache.org by rajeshbalamohan <gi...@git.apache.org> on 2017/11/03 05:00:51 UTC
[GitHub] orc pull request #186: ORC-187: BitFieldReader has an unnecessary loop
GitHub user rajeshbalamohan opened a pull request:
https://github.com/apache/orc/pull/186
ORC-187: BitFieldReader has an unnecessary loop
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/rajeshbalamohan/orc ORC-187
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/orc/pull/186.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 #186
----
commit 2c91d132a0e66bfa4cf7c95ab20f2de7a6326ff0
Author: Rajesh Balamohan <rb...@apache.org>
Date: 2017-11-03T04:58:28Z
ORC-187: BitFieldReader has an unnecessary loop
----
---
[GitHub] orc pull request #186: ORC-187: BitFieldReader has an unnecessary loop
Posted by t3rmin4t0r <gi...@git.apache.org>.
Github user t3rmin4t0r commented on a diff in the pull request:
https://github.com/apache/orc/pull/186#discussion_r148716091
--- Diff: java/core/src/java/org/apache/orc/impl/BitFieldReader.java ---
@@ -162,53 +92,32 @@ public void seek(PositionProvider index) throws IOException {
consumed + " in " + input);
} else if (consumed != 0) {
readByte();
- bitsLeft = 8 - consumed;
+ currentIdx = (byte) consumed;
} else {
- bitsLeft = 0;
+ currentIdx = 8;
}
}
public void skip(long items) throws IOException {
- long totalBits = bitSize * items;
- if (bitsLeft >= totalBits) {
- bitsLeft -= totalBits;
+ final long totalBits = bitSize * items;
+ final int availableBits = 8 - (currentIdx + 1);
+ if (totalBits <= availableBits) {
+ currentIdx += totalBits;
} else {
- totalBits -= bitsLeft;
- input.skip(totalBits / 8);
- current = input.next();
- bitsLeft = (int) (8 - (totalBits % 8));
+ final long bitsToSkip = (totalBits - availableBits);
+ input.skip(Math.min(1, bitsToSkip / 8));
--- End diff --
Looks a bit odd - the min(1) might be a problem if
availableBits = 0 and items=3, it will end up skip(1) where it should really be doing skip(0).
---
[GitHub] orc pull request #186: ORC-187: Simplify BitFieldReader to only support sing...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/orc/pull/186
---
[GitHub] orc issue #186: ORC-187: BitFieldReader has an unnecessary loop
Posted by t3rmin4t0r <gi...@git.apache.org>.
Github user t3rmin4t0r commented on the issue:
https://github.com/apache/orc/pull/186
LGTM - +1
---
[GitHub] orc issue #186: ORC-187: Simplify BitFieldReader to only support single bits
Posted by t3rmin4t0r <gi...@git.apache.org>.
Github user t3rmin4t0r commented on the issue:
https://github.com/apache/orc/pull/186
@asfgit merge
---
[GitHub] orc issue #186: ORC-187: BitFieldReader has an unnecessary loop
Posted by omalley <gi...@git.apache.org>.
Github user omalley commented on the issue:
https://github.com/apache/orc/pull/186
Since this is removing the old functionality, you should remove the old constructor that takes the bitWidth. With this change, someone who passed in a non-one width would be surprised with the result.
---