ctubbsii commented on code in PR #3219:
URL: https://github.com/apache/accumulo/pull/3219#discussion_r1123787883
##########
core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/SeekableByteArrayInputStream.java:
##########
@@ -42,13 +43,13 @@ public class SeekableByteArrayInputStream extends
InputStream {
@SuppressFBWarnings(value = "VO_VOLATILE_REFERENCE_TO_ARRAY",
justification = "see explanation above")
private volatile byte[] buffer;
- private int cur;
- private int max;
+ private final AtomicInteger cur;
+ private final int max;
@Override
public int read() {
- if (cur < max) {
- return buffer[cur++] & 0xff;
+ if (cur.get() < max) {
+ return buffer[cur.getAndIncrement()] & 0xff;
} else {
return -1;
}
Review Comment:
I don't know if this method needs to be thread-safe, but it does seem odd to
call `get` and then `getAndIncrement` in the next line, when the two gets could
be different in this construction, if multiple threads called this method (or
another one that changed the pointer).
I think you could change this to do the get and the increment atomically.
Maybe something like this:
```suggestion
int currentValue = cur.getAndAccumulate(1, (x, i) -> x < max ? x + i :
x);
if (currentValue < max) {
return buffer[currentValue] & 0xff;
} else {
return -1;
}
```
However, I don't think that it would be enough to make this class
thread-safe.
##########
core/src/main/java/org/apache/accumulo/core/file/blockfile/impl/SeekableByteArrayInputStream.java:
##########
@@ -42,13 +43,13 @@ public class SeekableByteArrayInputStream extends
InputStream {
@SuppressFBWarnings(value = "VO_VOLATILE_REFERENCE_TO_ARRAY",
justification = "see explanation above")
private volatile byte[] buffer;
- private int cur;
- private int max;
+ private final AtomicInteger cur;
Review Comment:
In both constructors, you're initializing it to 0. You can just do that here
once.
```suggestion
private final AtomicInteger cur = new AtomicInteger(0);
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
