From 980effaedeea8e81322a2a7b836834a9a7a0883c Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Tue, 13 Aug 2013 11:34:05 -0700 Subject: [PATCH] WebSocket - reducing memory footprint of WebSocketFrame by not using boolean fields --- .../test/resources/jetty-logging.properties | 2 +- .../jetty/websocket/api/extensions/Frame.java | 1 + .../jetty/websocket/common/Generator.java | 2 +- .../websocket/common/WebSocketFrame.java | 127 ++++++++---------- .../websocket/common/WebSocketFrameTest.java | 72 +++++++--- .../websocket/server/ab/TestABCase4.java | 2 +- .../websocket/server/ab/TestABCase7.java | 41 ------ .../test/resources/jetty-logging.properties | 2 +- 8 files changed, 111 insertions(+), 138 deletions(-) diff --git a/jetty-websocket/javax-websocket-server-impl/src/test/resources/jetty-logging.properties b/jetty-websocket/javax-websocket-server-impl/src/test/resources/jetty-logging.properties index d091aab3a83..88b96eead11 100644 --- a/jetty-websocket/javax-websocket-server-impl/src/test/resources/jetty-logging.properties +++ b/jetty-websocket/javax-websocket-server-impl/src/test/resources/jetty-logging.properties @@ -2,7 +2,7 @@ org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog org.eclipse.jetty.LEVEL=WARN # org.eclipse.jetty.websocket.LEVEL=DEBUG -org.eclipse.jetty.websocket.LEVEL=INFO +# org.eclipse.jetty.websocket.LEVEL=INFO # org.eclipse.jetty.websocket.LEVEL=WARN # org.eclipse.jetty.websocket.common.io.LEVEL=DEBUG diff --git a/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/extensions/Frame.java b/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/extensions/Frame.java index a1dae7530f2..ed2307f6ef6 100644 --- a/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/extensions/Frame.java +++ b/jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/extensions/Frame.java @@ -101,6 +101,7 @@ public interface Frame * * @return true if final frame. */ + // FIXME: remove public boolean isLast(); public boolean isMasked(); diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/Generator.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/Generator.java index 8bf4be3040c..6e1b7f12a35 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/Generator.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/Generator.java @@ -218,7 +218,7 @@ public class Generator } if (frame.isRsv3()) { - b |= 0x10; + b |= 0x10; // 0001_0000 } // NOTE: using .getOpCode() here, not .getType().getOpCode() for testing reasons diff --git a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketFrame.java b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketFrame.java index 569e0af44e5..323aa621e35 100644 --- a/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketFrame.java +++ b/jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/WebSocketFrame.java @@ -85,12 +85,19 @@ public class WebSocketFrame implements Frame return new WebSocketFrame(OpCode.TEXT).setPayload(msg); } - // FIXME: make each boolean/bit part of 1 byte (instead of multiple booleans) to save memory - private boolean fin = true; - private boolean rsv1 = false; - private boolean rsv2 = false; - private boolean rsv3 = false; - protected byte opcode = OpCode.UNDEFINED; + /** + * Combined FIN + RSV1 + RSV2 + RSV3 + OpCode byte. + *

+ * 

+     *   1000_0000 (0x80) = fin
+     *   0100_0000 (0x40) = rsv1
+     *   0010_0000 (0x20) = rsv2
+     *   0001_0000 (0x10) = rsv3
+     *   0000_1111 (0x0F) = opcode
+     *
+ */ + protected byte finRsvOp; + private boolean masked = false; private byte mask[]; /** @@ -138,11 +145,13 @@ public class WebSocketFrame implements Frame else { // Copy manually - fin = frame.isFin(); - rsv1 = frame.isRsv1(); - rsv2 = frame.isRsv2(); - rsv3 = frame.isRsv3(); - opcode = frame.getType().getOpCode(); + finRsvOp = 0x00; + finRsvOp |= frame.isFin() ? 0x80 : 0x00; + finRsvOp |= frame.isRsv1() ? 0x40 : 0x00; + finRsvOp |= frame.isRsv2() ? 0x20 : 0x00; + finRsvOp |= frame.isRsv3() ? 0x10 : 0x00; + finRsvOp |= frame.getOpCode() & 0x0F; + type = frame.getType(); masked = frame.isMasked(); mask = null; @@ -187,7 +196,7 @@ public class WebSocketFrame implements Frame public void assertValid() { - if (OpCode.isControlFrame(opcode)) + if (isControlFrame()) { if (getPayloadLength() > WebSocketFrame.MAX_CONTROL_PAYLOAD) { @@ -195,22 +204,22 @@ public class WebSocketFrame implements Frame + MAX_CONTROL_PAYLOAD + "]"); } - if (fin == false) + if ((finRsvOp & 0x80) == 0) { throw new ProtocolException("Cannot have FIN==false on Control frames"); } - if (rsv1 == true) + if ((finRsvOp & 0x40) != 0) { throw new ProtocolException("Cannot have RSV1==true on Control frames"); } - if (rsv2 == true) + if ((finRsvOp & 0x20) != 0) { throw new ProtocolException("Cannot have RSV2==true on Control frames"); } - if (rsv3 == true) + if ((finRsvOp & 0x10) != 0) { throw new ProtocolException("Cannot have RSV3==true on Control frames"); } @@ -224,11 +233,7 @@ public class WebSocketFrame implements Frame private final void copy(WebSocketFrame copy, ByteBuffer payload) { - fin = copy.fin; - rsv1 = copy.rsv1; - rsv2 = copy.rsv2; - rsv3 = copy.rsv3; - opcode = copy.opcode; + finRsvOp = copy.finRsvOp; type = copy.type; masked = copy.masked; mask = null; @@ -278,7 +283,7 @@ public class WebSocketFrame implements Frame { return false; } - if (fin != other.fin) + if (finRsvOp != other.finRsvOp) { return false; } @@ -290,22 +295,6 @@ public class WebSocketFrame implements Frame { return false; } - if (opcode != other.opcode) - { - return false; - } - if (rsv1 != other.rsv1) - { - return false; - } - if (rsv2 != other.rsv2) - { - return false; - } - if (rsv3 != other.rsv3) - { - return false; - } return true; } @@ -336,7 +325,7 @@ public class WebSocketFrame implements Frame @Override public final byte getOpCode() { - return opcode; + return (byte)(finRsvOp & 0x0F); } /** @@ -386,13 +375,8 @@ public class WebSocketFrame implements Frame result = (prime * result) + (continuation?1231:1237); result = (prime * result) + continuationIndex; result = (prime * result) + ((data == null)?0:data.hashCode()); - result = (prime * result) + (fin?1231:1237); + result = (prime * result) + finRsvOp; result = (prime * result) + Arrays.hashCode(mask); - result = (prime * result) + (masked?1231:1237); - result = (prime * result) + opcode; - result = (prime * result) + (rsv1?1231:1237); - result = (prime * result) + (rsv2?1231:1237); - result = (prime * result) + (rsv3?1231:1237); return result; } @@ -410,29 +394,30 @@ public class WebSocketFrame implements Frame public boolean isControlFrame() { - return OpCode.isControlFrame(opcode); + return OpCode.isControlFrame(getOpCode()); } public boolean isDataFrame() { - return OpCode.isDataFrame(opcode); + return OpCode.isDataFrame(getOpCode()); } @Override public boolean isFin() { - return fin; + return (byte)(finRsvOp & 0x80) != 0; } @Override public boolean isLast() { - return fin; + return isFin(); } + // FIXME: remove public boolean isLastFrame() { - return fin; + return isFin(); } @Override @@ -444,19 +429,19 @@ public class WebSocketFrame implements Frame @Override public boolean isRsv1() { - return rsv1; + return (byte)(finRsvOp & 0x40) != 0; } @Override public boolean isRsv2() { - return rsv2; + return (byte)(finRsvOp & 0x20) != 0; } @Override public boolean isRsv3() { - return rsv3; + return (byte)(finRsvOp & 0x10) != 0; } /** @@ -494,11 +479,7 @@ public class WebSocketFrame implements Frame public void reset() { - fin = true; - rsv1 = false; - rsv2 = false; - rsv3 = false; - opcode = -1; + finRsvOp = (byte) 0x80; // FIN (!RSV, opcode 0) masked = false; data = null; payloadLength = 0; @@ -521,7 +502,8 @@ public class WebSocketFrame implements Frame public WebSocketFrame setFin(boolean fin) { - this.fin = fin; + // set bit 1 + this.finRsvOp = (byte)((finRsvOp & 0x7F) | (fin? 0x80:0x00)); return this; } @@ -540,7 +522,7 @@ public class WebSocketFrame implements Frame public WebSocketFrame setOpCode(byte op) { - this.opcode = op; + this.finRsvOp = (byte)((finRsvOp & 0xF0) | (op & 0x0F)); if (op == OpCode.UNDEFINED) { @@ -567,7 +549,7 @@ public class WebSocketFrame implements Frame return this; } - if (OpCode.isControlFrame(opcode)) + if (isControlFrame()) { if (buf.length > WebSocketFrame.MAX_CONTROL_PAYLOAD) { @@ -594,7 +576,7 @@ public class WebSocketFrame implements Frame return this; } - if (OpCode.isControlFrame(opcode)) + if (isControlFrame()) { if (len > WebSocketFrame.MAX_CONTROL_PAYLOAD) { @@ -625,7 +607,7 @@ public class WebSocketFrame implements Frame return this; } - if (OpCode.isControlFrame(opcode)) + if (isControlFrame()) { if (buf.remaining() > WebSocketFrame.MAX_CONTROL_PAYLOAD) { @@ -646,19 +628,22 @@ public class WebSocketFrame implements Frame public WebSocketFrame setRsv1(boolean rsv1) { - this.rsv1 = rsv1; + // set bit 2 + this.finRsvOp = (byte)((finRsvOp & 0xBF) | (rsv1? 0x40:0x00)); return this; } public WebSocketFrame setRsv2(boolean rsv2) { - this.rsv2 = rsv2; + // set bit 3 + this.finRsvOp = (byte)((finRsvOp & 0xDF) | (rsv2? 0x20:0x00)); return this; } public WebSocketFrame setRsv3(boolean rsv3) { - this.rsv3 = rsv3; + // set bit 4 + this.finRsvOp = (byte)((finRsvOp & 0xEF) | (rsv3? 0x10:0x00)); return this; } @@ -666,14 +651,14 @@ public class WebSocketFrame implements Frame public String toString() { StringBuilder b = new StringBuilder(); - b.append(OpCode.name(opcode)); + b.append(OpCode.name((byte)(finRsvOp & 0x0F))); b.append('['); b.append("len=").append(payloadLength); - b.append(",fin=").append(fin); + b.append(",fin=").append((finRsvOp & 0x80)!=0); b.append(",rsv="); - b.append(rsv1?'1':'.'); - b.append(rsv2?'1':'.'); - b.append(rsv3?'1':'.'); + b.append(((finRsvOp&0x40)!=0)?'1':'.'); + b.append(((finRsvOp&0x20)!=0)?'1':'.'); + b.append(((finRsvOp&0x10)!=0)?'1':'.'); b.append(",masked=").append(masked); b.append(",continuation=").append(continuation); b.append(",remaining=").append(remaining()); diff --git a/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/WebSocketFrameTest.java b/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/WebSocketFrameTest.java index 2b0c120d307..9bea559f59a 100644 --- a/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/WebSocketFrameTest.java +++ b/jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/WebSocketFrameTest.java @@ -18,6 +18,8 @@ package org.eclipse.jetty.websocket.common; +import static org.hamcrest.Matchers.*; + import java.nio.ByteBuffer; import org.eclipse.jetty.io.ByteBufferPool; @@ -26,6 +28,7 @@ import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.websocket.api.StatusCode; import org.eclipse.jetty.websocket.api.WebSocketPolicy; import org.eclipse.jetty.websocket.api.extensions.Frame; +import org.junit.Assert; import org.junit.BeforeClass; import org.junit.Test; @@ -58,16 +61,19 @@ public class WebSocketFrameTest ByteBufferAssert.assertEquals(message,expected,actual); } + private void assertFrameHex(String message, String expectedHex, ByteBuffer actual) + { + String actualHex = Hex.asHex(actual); + Assert.assertThat("Generated Frame:" + message,actualHex,is(expectedHex)); + } + @Test public void testLaxInvalidClose() { WebSocketFrame frame = new WebSocketFrame(OpCode.CLOSE).setFin(false); ByteBuffer actual = generateWholeFrame(laxGenerator,frame); - ByteBuffer expected = ByteBuffer.allocate(2); - expected.put((byte)0x08); - expected.put((byte)0x00); - - assertEqual("Lax Invalid Close Frame",expected,actual); + String expected = "0800"; + assertFrameHex("Lax Invalid Close Frame",expected,actual); } @Test @@ -75,11 +81,8 @@ public class WebSocketFrameTest { WebSocketFrame frame = new WebSocketFrame(OpCode.PING).setFin(false); ByteBuffer actual = generateWholeFrame(laxGenerator,frame); - ByteBuffer expected = ByteBuffer.allocate(2); - expected.put((byte)0x09); - expected.put((byte)0x00); - - assertEqual("Lax Invalid Ping Frame",expected,actual); + String expected = "0900"; + assertFrameHex("Lax Invalid Ping Frame",expected,actual); } @Test @@ -87,13 +90,8 @@ public class WebSocketFrameTest { CloseInfo close = new CloseInfo(StatusCode.NORMAL); ByteBuffer actual = generateWholeFrame(strictGenerator,close.asFrame()); - ByteBuffer expected = ByteBuffer.allocate(4); - expected.put((byte)0x88); - expected.put((byte)0x02); - expected.put((byte)0x03); - expected.put((byte)0xE8); - - assertEqual("Strict Valid Close Frame",expected,actual); + String expected = "880203E8"; + assertFrameHex("Strict Valid Close Frame",expected,actual); } @Test @@ -101,10 +99,40 @@ public class WebSocketFrameTest { WebSocketFrame frame = new WebSocketFrame(OpCode.PING); ByteBuffer actual = generateWholeFrame(strictGenerator,frame); - ByteBuffer expected = ByteBuffer.allocate(2); - expected.put((byte)0x89); - expected.put((byte)0x00); - - assertEqual("Strict Valid Ping Frame",expected,actual); + String expected = "8900"; + assertFrameHex("Strict Valid Ping Frame",expected,actual); + } + + @Test + public void testRsv1() + { + WebSocketFrame frame = new WebSocketFrame(OpCode.TEXT); + frame.setPayload("Hi"); + frame.setRsv1(true); + ByteBuffer actual = generateWholeFrame(laxGenerator,frame); + String expected = "C1024869"; + assertFrameHex("Lax Text Frame with RSV1",expected,actual); + } + + @Test + public void testRsv2() + { + WebSocketFrame frame = new WebSocketFrame(OpCode.TEXT); + frame.setPayload("Hi"); + frame.setRsv2(true); + ByteBuffer actual = generateWholeFrame(laxGenerator,frame); + String expected = "A1024869"; + assertFrameHex("Lax Text Frame with RSV2",expected,actual); + } + + @Test + public void testRsv3() + { + WebSocketFrame frame = new WebSocketFrame(OpCode.TEXT); + frame.setPayload("Hi"); + frame.setRsv3(true); + ByteBuffer actual = generateWholeFrame(laxGenerator,frame); + String expected = "91024869"; + assertFrameHex("Lax Text Frame with RSV3",expected,actual); } } diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase4.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase4.java index 01bfd2fe790..40736a89834 100644 --- a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase4.java +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase4.java @@ -44,7 +44,7 @@ public class TestABCase4 extends AbstractABCase public BadFrame(byte opcode) { super(); - super.opcode = opcode; + super.finRsvOp = (byte)((finRsvOp & 0xF0) | (opcode & 0x0F)); // NOTE: Not setting Frame.Type intentionally } } diff --git a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase7.java b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase7.java index 8cb96f9e5ee..1b1962ef44a 100644 --- a/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase7.java +++ b/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ab/TestABCase7.java @@ -361,47 +361,6 @@ public class TestABCase7 extends AbstractABCase } } - /** - * close with invalid payload (124 byte reason) (exceeds total allowed control frame payload bytes) - */ - @Test - public void testCase7_3_6() throws Exception - { - ByteBuffer payload = ByteBuffer.allocate(256); - BufferUtil.clearToFill(payload); - payload.put((byte)0xE8); - payload.put((byte)0x03); - byte reason[] = new byte[124]; // too big - Arrays.fill(reason,(byte)'!'); - payload.put(reason); - BufferUtil.flipToFlush(payload,0); - - List send = new ArrayList<>(); - WebSocketFrame close = new WebSocketFrame(); - close.setPayload(payload); - close.setOpCode(OpCode.CLOSE); // set opcode after payload (to prevent early bad payload detection) - send.add(close); - - List expect = new ArrayList<>(); - expect.add(new CloseInfo(StatusCode.PROTOCOL).asFrame()); - - Fuzzer fuzzer = new Fuzzer(this); - try - { - enableStacks(Parser.class,false); - fuzzer.connect(); - fuzzer.setSendMode(Fuzzer.SendMode.BULK); - fuzzer.send(send); - fuzzer.expect(expect); - fuzzer.expectNoMoreFrames(); - } - finally - { - enableStacks(Parser.class,true); - fuzzer.close(); - } - } - /** * close with invalid UTF8 in payload */ diff --git a/jetty-websocket/websocket-server/src/test/resources/jetty-logging.properties b/jetty-websocket/websocket-server/src/test/resources/jetty-logging.properties index 64b6532fcad..f5ce4698168 100644 --- a/jetty-websocket/websocket-server/src/test/resources/jetty-logging.properties +++ b/jetty-websocket/websocket-server/src/test/resources/jetty-logging.properties @@ -2,7 +2,7 @@ org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog org.eclipse.jetty.LEVEL=WARN # org.eclipse.jetty.io.WriteFlusher.LEVEL=DEBUG -org.eclipse.jetty.websocket.LEVEL=DEBUG +# org.eclipse.jetty.websocket.LEVEL=DEBUG # org.eclipse.jetty.websocket.LEVEL=WARN # org.eclipse.jetty.websocket.common.io.LEVEL=DEBUG # org.eclipse.jetty.websocket.server.ab.LEVEL=DEBUG