Skip to content

Commit

Permalink
Polishing #1367
Browse files Browse the repository at this point in the history
Rename calculateStringBytes to sizeOf. Eagerly obtain averageBytesPerChar/maxBytesPerChar as these values are constant for a given encoding so that we don't need to obtain CharsetEncoder for each size estimation. Reorder methods.

Original pull request: #1368.
  • Loading branch information
mp911de committed Jul 30, 2020
1 parent f17ed97 commit afb847d
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 54 deletions.
103 changes: 56 additions & 47 deletions src/main/java/io/lettuce/core/codec/StringCodec.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
* {@link Charset}. It accepts provided {@link ByteBuf buffers} so it does not need to allocate buffers during encoding.
*
* @author Mark Paluch
* @author Dimitris Mandalidis
* @since 4.3
*/
public class StringCodec implements RedisCodec<String, String>, ToByteBufEncoder<String, String> {
Expand All @@ -42,6 +43,10 @@ public class StringCodec implements RedisCodec<String, String>, ToByteBufEncoder

private final Charset charset;

private final float averageBytesPerChar;

private final float maxBytesPerChar;

private final boolean ascii;

private final boolean utf8;
Expand All @@ -65,6 +70,10 @@ public StringCodec(Charset charset) {

this.charset = charset;

CharsetEncoder encoder = CharsetUtil.encoder(charset);
this.averageBytesPerChar = encoder.averageBytesPerChar();
this.maxBytesPerChar = encoder.maxBytesPerChar();

if (charset.name().equals("UTF-8")) {
utf8 = true;
ascii = false;
Expand All @@ -82,47 +91,11 @@ public void encodeKey(String key, ByteBuf target) {
encode(key, target);
}

public void encode(String str, ByteBuf target) {

if (str == null) {
return;
}

if (utf8) {
ByteBufUtil.writeUtf8(target, str);
return;
}

if (ascii) {
ByteBufUtil.writeAscii(target, str);
return;
}

int length = calculateStringBytes(str, false);
target.ensureWritable(length);
CharsetEncoder encoder = CharsetUtil.encoder(charset);
try {
final ByteBuffer dstBuf = target.nioBuffer(0, length);
final int pos = dstBuf.position();
CoderResult cr = encoder.encode(CharBuffer.wrap(str), dstBuf, true);
if (!cr.isUnderflow()) {
cr.throwException();
}
cr = encoder.flush(dstBuf);
if (!cr.isUnderflow()) {
cr.throwException();
}
target.writerIndex(target.writerIndex() + dstBuf.position() - pos);
} catch (CharacterCodingException x) {
throw new IllegalStateException(x);
}
}

@Override
public int estimateSize(Object keyOrValue) {

if (keyOrValue instanceof String) {
return calculateStringBytes((String) keyOrValue, true);
return sizeOf((String) keyOrValue, true);
}
return 0;
}
Expand Down Expand Up @@ -159,11 +132,12 @@ public ByteBuffer encodeValue(String value) {
* @return
*/
private ByteBuffer encodeAndAllocateBuffer(String key) {

if (key == null) {
return ByteBuffer.wrap(EMPTY);
}

ByteBuffer buffer = ByteBuffer.allocate(calculateStringBytes(key, false));
ByteBuffer buffer = ByteBuffer.allocate(sizeOf(key, false));

ByteBuf byteBuf = Unpooled.wrappedBuffer(buffer);
byteBuf.clear();
Expand All @@ -172,20 +146,55 @@ private ByteBuffer encodeAndAllocateBuffer(String key) {

return buffer;
}


public void encode(String str, ByteBuf target) {

if (str == null) {
return;
}

if (utf8) {
ByteBufUtil.writeUtf8(target, str);
return;
}

if (ascii) {
ByteBufUtil.writeAscii(target, str);
return;
}

CharsetEncoder encoder = CharsetUtil.encoder(charset);
int length = sizeOf(str, false);
target.ensureWritable(length);

try {
ByteBuffer dstBuf = target.nioBuffer(0, length);
int pos = dstBuf.position();

CoderResult cr = encoder.encode(CharBuffer.wrap(str), dstBuf, true);
if (!cr.isUnderflow()) {
cr.throwException();
}
cr = encoder.flush(dstBuf);
if (!cr.isUnderflow()) {
cr.throwException();
}
target.writerIndex(target.writerIndex() + dstBuf.position() - pos);
} catch (CharacterCodingException x) {
throw new IllegalStateException(x);
}
}

/**
* Calculate either the maximum number of bytes a string may occupy in a given character set or
* the average number of bytes it may hold.
* @param encoder the character set encoder (from which char to byte count association is inferred)
* @param value the actual value (must be not null)
* @param estimate whether the caller needs for an estimation or an actual value needed by buffer allocation
* @return the calculated string byte count
*/
int calculateStringBytes(String value, boolean estimate) {
CharsetEncoder encoder = CharsetUtil.encoder(charset);
int sizeOf(String value, boolean estimate) {

if (estimate) {
return (int) (encoder.averageBytesPerChar() * value.length());
return (int) (averageBytesPerChar * value.length());
}
return (int) encoder.maxBytesPerChar() * value.length();

return (int) (maxBytesPerChar * value.length());
}
}
16 changes: 9 additions & 7 deletions src/test/java/io/lettuce/core/codec/StringCodecUnitTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,17 +18,18 @@
import static org.assertj.core.api.Assertions.assertThat;

import java.nio.ByteBuffer;
import java.nio.charset.CharsetEncoder;
import java.nio.charset.StandardCharsets;

import org.junit.jupiter.api.Test;

import io.netty.buffer.ByteBuf;
import io.netty.buffer.Unpooled;
import io.netty.util.CharsetUtil;

/**
* Unit tests for {@link StringCodec}.
*
* @author Mark Paluch
* @author Dimitris Mandalidis
*/
class StringCodecUnitTests {

Expand Down Expand Up @@ -118,14 +119,15 @@ void estimateSize() {
assertThat(new StringCodec(StandardCharsets.US_ASCII).estimateSize(teststring)).isEqualTo(teststring.length());
assertThat(new StringCodec(StandardCharsets.ISO_8859_1).estimateSize(teststring)).isEqualTo(teststring.length());
}

@Test
public void calculateStringSize() {
assertThat(new StringCodec(StandardCharsets.UTF_8).calculateStringBytes(teststring, false))
void sizeOf() {

assertThat(new StringCodec(StandardCharsets.UTF_8).sizeOf(teststring, false))
.isEqualTo(teststring.length() * 3);
assertThat(new StringCodec(StandardCharsets.US_ASCII).calculateStringBytes(teststring, false))
assertThat(new StringCodec(StandardCharsets.US_ASCII).sizeOf(teststring, false))
.isEqualTo(teststring.length());
assertThat(new StringCodec(StandardCharsets.ISO_8859_1).calculateStringBytes(teststring, false))
assertThat(new StringCodec(StandardCharsets.ISO_8859_1).sizeOf(teststring, false))
.isEqualTo(teststring.length());
}
}

0 comments on commit afb847d

Please sign in to comment.