Skip to content

Commit 9857aeb

Browse files
authored
Merge pull request #189 from fanson/perf/zero-alloc-parsing
perf: cache command string and add zero-allocation integer parsing
2 parents 59c643b + 8c84bfe commit 9857aeb

File tree

5 files changed

+85
-8
lines changed

5 files changed

+85
-8
lines changed

src/main/java/com/github/tonivade/resp/command/DefaultRequest.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111

1212
public class DefaultRequest implements Request {
1313

14-
private final SafeString command;
14+
private final String command;
1515

1616
private final List<SafeString> params;
1717

@@ -22,13 +22,13 @@ public class DefaultRequest implements Request {
2222
public DefaultRequest(ServerContext server, Session session, SafeString command, List<SafeString> params) {
2323
this.server = server;
2424
this.session = session;
25-
this.command = checkNonNull(command);
25+
this.command = checkNonNull(command).toString();
2626
this.params = checkNonNull(params);
2727
}
2828

2929
@Override
3030
public String getCommand() {
31-
return command.toString();
31+
return command;
3232
}
3333

3434
@Override
@@ -61,7 +61,7 @@ public boolean isEmpty() {
6161

6262
@Override
6363
public boolean isExit() {
64-
return command.toString().equalsIgnoreCase("quit");
64+
return getCommand().equalsIgnoreCase("quit");
6565
}
6666

6767
@Override

src/main/java/com/github/tonivade/resp/protocol/RedisParser.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,7 @@ private RedisToken parseToken(SafeString line) {
6060
RedisToken token = new UnknownRedisToken(SafeString.EMPTY_STRING);
6161
if (line != null && !line.isEmpty()) {
6262
if (line.startsWith(ARRAY_PREFIX)) {
63-
int size = Integer.parseInt(line.substring(1));
63+
int size = line.parseIntAfterPrefix();
6464
token = parseArray(size);
6565
} else if (line.startsWith(STATUS_PREFIX)) {
6666
token = status(line.substring(1));
@@ -78,13 +78,12 @@ private RedisToken parseToken(SafeString line) {
7878
}
7979

8080
private RedisToken parseIntegerToken(SafeString line) {
81-
Integer value = Integer.valueOf(line.substring(1));
82-
return new IntegerRedisToken(value);
81+
return new IntegerRedisToken(line.parseIntAfterPrefix());
8382
}
8483

8584
private RedisToken parseStringToken(SafeString line) {
8685
StringRedisToken token;
87-
int length = Integer.parseInt(line.substring(1));
86+
int length = line.parseIntAfterPrefix();
8887
if (length >= 0 && length < maxLength) {
8988
token = new StringRedisToken(source.readString(length));
9089
} else {

src/main/java/com/github/tonivade/resp/protocol/SafeString.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,26 @@ public String substring(int i) {
120120
return toString().substring(i);
121121
}
122122

123+
public int parseIntAfterPrefix() {
124+
int pos = buffer.position();
125+
int lim = buffer.limit();
126+
boolean negative = false;
127+
int result = 0;
128+
int start = pos + 1;
129+
if (start < lim && buffer.get(start) == '-') {
130+
negative = true;
131+
start++;
132+
}
133+
for (int i = start; i < lim; i++) {
134+
byte b = buffer.get(i);
135+
if (b < '0' || b > '9') {
136+
throw new NumberFormatException("Invalid character in number: " + (char) b);
137+
}
138+
result = result * 10 + (b - '0');
139+
}
140+
return negative ? -result : result;
141+
}
142+
123143
private static int compare(byte[] left, byte[] right) {
124144
for (int i = 0, j = 0; i < left.length && j < right.length; i++, j++) {
125145
int a = (left[i] & 0xff);

src/test/java/com/github/tonivade/resp/command/DefaultRequestTest.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
import static com.github.tonivade.resp.protocol.SafeString.safeString;
99
import static org.hamcrest.CoreMatchers.is;
1010
import static org.hamcrest.CoreMatchers.nullValue;
11+
import static org.hamcrest.CoreMatchers.sameInstance;
1112
import static org.hamcrest.MatcherAssert.assertThat;
1213

1314
import org.junit.jupiter.api.Test;
@@ -31,4 +32,27 @@ void testRequest() {
3132
assertThat(request.getOptionalParam(3).isPresent(), is(false));
3233
assertThat(request.toString(), is("a[3]: [1, 2, 3]"));
3334
}
35+
36+
@Test
37+
void getCommandReturnsCachedInstance() {
38+
DefaultRequest request = new DefaultRequest(null, null, safeString("PING"),
39+
safeAsList());
40+
41+
String first = request.getCommand();
42+
String second = request.getCommand();
43+
44+
assertThat(first, is("PING"));
45+
assertThat(second, sameInstance(first));
46+
}
47+
48+
@Test
49+
void isExitUsesCache() {
50+
DefaultRequest quit = new DefaultRequest(null, null, safeString("QUIT"),
51+
safeAsList());
52+
DefaultRequest ping = new DefaultRequest(null, null, safeString("PING"),
53+
safeAsList());
54+
55+
assertThat(quit.isExit(), is(true));
56+
assertThat(ping.isExit(), is(false));
57+
}
3458
}

src/test/java/com/github/tonivade/resp/protocol/SafeStringTest.java

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import static com.github.tonivade.resp.protocol.SafeString.safeString;
1010
import static org.hamcrest.CoreMatchers.is;
1111
import static org.hamcrest.MatcherAssert.assertThat;
12+
import static org.junit.jupiter.api.Assertions.assertThrows;
1213

1314
import java.util.Iterator;
1415
import java.util.List;
@@ -41,6 +42,39 @@ void testList() {
4142
assertThat(list.get(2), is(safeString("3")));
4243
}
4344

45+
@Test
46+
void parseIntAfterPrefixPositive() {
47+
assertThat(safeString("*3").parseIntAfterPrefix(), is(3));
48+
assertThat(safeString("$123").parseIntAfterPrefix(), is(123));
49+
assertThat(safeString(":42").parseIntAfterPrefix(), is(42));
50+
}
51+
52+
@Test
53+
void parseIntAfterPrefixNegative() {
54+
assertThat(safeString(":-1").parseIntAfterPrefix(), is(-1));
55+
assertThat(safeString("$-1").parseIntAfterPrefix(), is(-1));
56+
assertThat(safeString(":-999").parseIntAfterPrefix(), is(-999));
57+
}
58+
59+
@Test
60+
void parseIntAfterPrefixZero() {
61+
assertThat(safeString("$0").parseIntAfterPrefix(), is(0));
62+
assertThat(safeString(":0").parseIntAfterPrefix(), is(0));
63+
}
64+
65+
@Test
66+
void parseIntAfterPrefixLargeNumber() {
67+
assertThat(safeString("*100000").parseIntAfterPrefix(), is(100000));
68+
assertThat(safeString(":2147483647").parseIntAfterPrefix(), is(Integer.MAX_VALUE));
69+
}
70+
71+
@Test
72+
void parseIntAfterPrefixInvalidCharacter() {
73+
assertThrows(NumberFormatException.class, () -> safeString("*abc").parseIntAfterPrefix());
74+
assertThrows(NumberFormatException.class, () -> safeString("$12x").parseIntAfterPrefix());
75+
assertThrows(NumberFormatException.class, () -> safeString(":1.5").parseIntAfterPrefix());
76+
}
77+
4478
@Test
4579
void testSet() {
4680
NavigableSet<SafeString> set = new TreeSet<>(safeAsList("1", "2", "3"));

0 commit comments

Comments
 (0)