Repository: relayrides/pushy
Pull request #289 (2016-05-05 03:08:28)

Further efficient-ize ApnsPayloadBuilder
This does two things to make `ApnsPayloadBuilder` a bit more efficient:

1. It introduces a reusable buffer for JSON generation, which should cut down on buffer allocations and reduce GC pressure when reusing `ApnsPayloadBuilder` instances.
2. It totally changes the algorithm for finding alert bodies that fit within a limited payload size. Previously, we were doing a binary search. In c9c9b89, we just count bytes to figure out how much of the alert body we can include.

Benchmarks:

```
Before:

(messageBodyLength)      (unicodeBlockName)   Mode  Cnt       Score      Error  Units
                512             BASIC_LATIN  thrpt   80  222343.928 ± 4858.257  ops/s
                512  CJK_UNIFIED_IDEOGRAPHS  thrpt   80  305840.679 ± 1133.963  ops/s
               8192             BASIC_LATIN  thrpt   80    3323.071 ±  238.083  ops/s
               8192  CJK_UNIFIED_IDEOGRAPHS  thrpt   80    6939.819 ±   25.177  ops/s

After:

(messageBodyLength)      (unicodeBlockName)   Mode  Cnt       Score      Error  Units
                512             BASIC_LATIN  thrpt   80  222640.631 ± 1577.262  ops/s
                512  CJK_UNIFIED_IDEOGRAPHS  thrpt   80  331607.892 ± 1820.027  ops/s
               8192             BASIC_LATIN  thrpt   80    9927.099 ±   69.721  ops/s
               8192  CJK_UNIFIED_IDEOGRAPHS  thrpt   80   21185.339 ±   83.509  ops/s
```


Patch:
diff --git a/pushy/src/main/java/com/relayrides/pushy/apns/ApnsClientHandler.java b/pushy/src/main/java/com/relayrides/pushy/apns/ApnsClientHandler.java
index 4dbef36a..29268a0d 100644
--- a/pushy/src/main/java/com/relayrides/pushy/apns/ApnsClientHandler.java
+++ b/pushy/src/main/java/com/relayrides/pushy/apns/ApnsClientHandler.java
@@ -20,7 +20,7 @@
 
 package com.relayrides.pushy.apns;
 
-import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.Map;
@@ -88,8 +88,6 @@ class ApnsClientHandler<T extends ApnsPushNotification> extends Http2ConnectionH
             .registerTypeAdapter(Date.class, new DateAsMillisecondsSinceEpochTypeAdapter())
             .create();
 
-    private static final Charset UTF8 = Charset.forName("UTF-8");
-
     private static final Logger log = LoggerFactory.getLogger(ApnsClientHandler.class);
 
     public static class ApnsClientHandlerBuilder<S extends ApnsPushNotification> extends AbstractHttp2ConnectionHandlerBuilder<ApnsClientHandler<S>, ApnsClientHandlerBuilder<S>> {
@@ -151,7 +149,7 @@ class ApnsClientHandler<T extends ApnsPushNotification> extends Http2ConnectionH
 
         @Override
         public int onDataRead(final ChannelHandlerContext context, final int streamId, final ByteBuf data, final int padding, final boolean endOfStream) throws Http2Exception {
-            log.trace("Received data from APNs gateway on stream {}: {}", streamId, data.toString(UTF8));
+            log.trace("Received data from APNs gateway on stream {}: {}", streamId, data.toString(StandardCharsets.UTF_8));
 
             final int bytesProcessed = data.readableBytes() + padding;
 
@@ -160,7 +158,7 @@ class ApnsClientHandler<T extends ApnsPushNotification> extends Http2ConnectionH
                 final T pushNotification = ApnsClientHandler.this.pushNotificationsByStreamId.remove(streamId);
 
                 final boolean success = HttpResponseStatus.OK.equals(HttpResponseStatus.parseLine(headers.status()));
-                final ErrorResponse errorResponse = gson.fromJson(data.toString(UTF8), ErrorResponse.class);
+                final ErrorResponse errorResponse = gson.fromJson(data.toString(StandardCharsets.UTF_8), ErrorResponse.class);
 
                 ApnsClientHandler.this.apnsClient.handlePushNotificationResponse(new SimplePushNotificationResponse<>(
                         pushNotification, success, errorResponse.getReason(), errorResponse.getTimestamp()));
@@ -208,7 +206,7 @@ class ApnsClientHandler<T extends ApnsPushNotification> extends Http2ConnectionH
 
         @Override
         public void onGoAwayRead(final ChannelHandlerContext context, final int lastStreamId, final long errorCode, final ByteBuf debugData) throws Http2Exception {
-            log.info("Received GOAWAY from APNs server: {}", debugData.toString(UTF8));
+            log.info("Received GOAWAY from APNs server: {}", debugData.toString(StandardCharsets.UTF_8));
         }
     }
 
@@ -246,7 +244,7 @@ class ApnsClientHandler<T extends ApnsPushNotification> extends Http2ConnectionH
             log.trace("Wrote headers on stream {}: {}", streamId, headers);
 
             final ByteBuf payloadBuffer = context.alloc().ioBuffer(INITIAL_PAYLOAD_BUFFER_CAPACITY);
-            payloadBuffer.writeBytes(pushNotification.getPayload().getBytes(UTF8));
+            payloadBuffer.writeBytes(pushNotification.getPayload().getBytes(StandardCharsets.UTF_8));
 
             final ChannelPromise dataPromise = context.newPromise();
             this.encoder().writeData(context, streamId, payloadBuffer, 0, true, dataPromise);
diff --git a/pushy/src/main/java/com/relayrides/pushy/apns/util/ApnsPayloadBuilder.java b/pushy/src/main/java/com/relayrides/pushy/apns/util/ApnsPayloadBuilder.java
index 3b78a485..b9194051 100644
--- a/pushy/src/main/java/com/relayrides/pushy/apns/util/ApnsPayloadBuilder.java
+++ b/pushy/src/main/java/com/relayrides/pushy/apns/util/ApnsPayloadBuilder.java
@@ -20,7 +20,8 @@
 
 package com.relayrides.pushy.apns.util;
 
-import java.nio.charset.Charset;
+import java.io.CharArrayWriter;
+import java.nio.charset.StandardCharsets;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Map;
@@ -54,6 +55,8 @@ public class ApnsPayloadBuilder {
     private String categoryName = null;
     private boolean contentAvailable = false;
 
+    private final CharArrayWriter buffer = new CharArrayWriter(DEFAULT_PAYLOAD_SIZE / 4);
+
     private static final String APS_KEY = "aps";
     private static final String ALERT_KEY = "alert";
     private static final String BADGE_KEY = "badge";
@@ -74,10 +77,9 @@ public class ApnsPayloadBuilder {
 
     private static final int DEFAULT_PAYLOAD_SIZE = 4096;
 
-    private static final Charset UTF8 = Charset.forName("UTF-8");
     private static final String ABBREVIATION_SUBSTRING = "…";
 
-    private static final Gson gson = new GsonBuilder().serializeNulls().create();
+    private static final Gson gson = new GsonBuilder().serializeNulls().disableHtmlEscaping().create();
 
     /**
      * The name of the iOS default push notification sound
@@ -401,8 +403,11 @@ public class ApnsPayloadBuilder {
             payload.put(entry.getKey(), entry.getValue());
         }
 
-        final String payloadString = gson.toJson(payload);
-        final int initialPayloadSize = payloadString.getBytes(UTF8).length;
+        this.buffer.reset();
+        gson.toJson(payload, this.buffer);
+
+        final String payloadString = this.buffer.toString();
+        final int initialPayloadSize = payloadString.getBytes(StandardCharsets.UTF_8).length;
 
         final String fittedPayloadString;
 
@@ -411,43 +416,28 @@ public class ApnsPayloadBuilder {
         } else {
             if (this.alertBody != null) {
                 this.replaceMessageBody(payload, "");
-                final int payloadSizeWithEmptyMessage = gson.toJson(payload).getBytes(UTF8).length;
+
+                this.buffer.reset();
+                gson.toJson(payload, this.buffer);
+
+                final int payloadSizeWithEmptyMessage = this.buffer.toString().getBytes(StandardCharsets.UTF_8).length;
 
                 if (payloadSizeWithEmptyMessage >= maximumPayloadSize) {
                     throw new IllegalArgumentException("Payload exceeds maximum size even with an empty message body.");
                 }
 
-                // We add 2 here because the escaped string will include opening and closing '"' characters that are
-                // already accounted for in payloadSizeWithEmptyMessage.
-                final int maximumEscapedMessageBodySize = maximumPayloadSize - payloadSizeWithEmptyMessage + 2;
-
-                // Characters in the message body may wind up representing different numbers of bytes as an escaped
-                // JSON string. Assuming a best case of one byte per character, we have a ceiling on the maximum number
-                // of characters we could possibly fit into the message body. We can also figure out a minimum number
-                // of characters; in the worst case (a unicode control character), a single character could expand to
-                // take up six bytes in the escaped JSON string.
-                int left = maximumEscapedMessageBodySize / 6;
-                int right = maximumEscapedMessageBodySize;
+                final int maximumEscapedMessageBodySize = maximumPayloadSize - payloadSizeWithEmptyMessage -
+                        ABBREVIATION_SUBSTRING.getBytes(StandardCharsets.UTF_8).length;
 
-                String fittedMessageBody = null;
+                final String fittedMessageBody = this.alertBody.substring(0,
+                        ApnsPayloadBuilder.getLengthOfJsonEscapedUtf8StringFittingSize(this.alertBody, maximumEscapedMessageBodySize));
 
-                while (left < right) {
-                    final int middle = (left + right) / 2;
+                this.replaceMessageBody(payload, fittedMessageBody + ABBREVIATION_SUBSTRING);
 
-                    fittedMessageBody = this.abbreviateString(this.alertBody, middle);
-                    final int escapedMessageBodySize = gson.toJson(fittedMessageBody).getBytes(UTF8).length;
+                this.buffer.reset();
+                gson.toJson(payload, this.buffer);
 
-                    if (escapedMessageBodySize == maximumEscapedMessageBodySize) {
-                        break;
-                    } else if (escapedMessageBodySize < maximumEscapedMessageBodySize) {
-                        left = middle + 1;
-                    } else {
-                        right = middle - 1;
-                    }
-                }
-
-                this.replaceMessageBody(payload, fittedMessageBody);
-                fittedPayloadString = gson.toJson(payload);
+                fittedPayloadString = this.buffer.toString();
             } else {
                 throw new IllegalArgumentException(String.format(
                         "Payload size is %d bytes (with a maximum of %d bytes) and cannot be shortened.",
@@ -481,22 +471,6 @@ public class ApnsPayloadBuilder {
         }
     }
 
-    private String abbreviateString(final String string, final int maximumLength) {
-        final String abbreviatedString;
-
-        if (string.length() <= maximumLength) {
-            abbreviatedString = string;
-        } else {
-            if (maximumLength <= ABBREVIATION_SUBSTRING.length()) {
-                throw new IllegalArgumentException("String is too short to abbreviate.");
-            }
-
-            abbreviatedString = string.substring(0, maximumLength - ABBREVIATION_SUBSTRING.length()) + ABBREVIATION_SUBSTRING;
-        }
-
-        return abbreviatedString;
-    }
-
     private Object createAlertObject() {
         if (this.hasAlertContent()) {
             if (this.shouldRepresentAlertAsString()) {
@@ -576,4 +550,53 @@ public class ApnsPayloadBuilder {
                 && this.localizedAlertTitleKey == null && this.localizedAlertKey == null
                 && this.localizedAlertArguments == null && this.localizedAlertTitleArguments == null;
     }
+
+    static int getLengthOfJsonEscapedUtf8StringFittingSize(final String string, final int maximumSize) {
+        int i = 0;
+        int cumulativeSize = 0;
+
+        for (i = 0; i < string.length(); i++) {
+            final char c = string.charAt(i);
+            final int charSize = getSizeOfJsonEscapedUtf8Character(c);
+
+            if (cumulativeSize + charSize > maximumSize) {
+                // The next character would put us over the edge; bail out here.
+                break;
+            }
+
+            cumulativeSize += charSize;
+
+            if (Character.isHighSurrogate(c)) {
+                // Skip the next character
+                i++;
+            }
+        }
+
+        return i;
+    }
+
+    static int getSizeOfJsonEscapedUtf8Character(char c) {
+        final int charSize;
+
+        if (c == '"' || c == '\\' || c == '\b' || c == '\f' || c == '\n' || c == '\r' || c == '\t') {
+            // Character is backslash-escaped in JSON
+            charSize = 2;
+        } else if (c <= 0x001F || c == '\u2028' || c == '\u2029') {
+            // Character will be represented as an escaped control character
+            charSize = 6;
+        } else {
+            // The character will be represented as an un-escaped UTF8 character
+            if (c <= 0x007F) {
+                charSize = 1;
+            } else if (c <= 0x07FF) {
+                charSize = 2;
+            } else if (Character.isHighSurrogate(c)) {
+                charSize = 4;
+            } else {
+                charSize = 3;
+            }
+        }
+
+        return charSize;
+    }
 }


Test patch:
diff --git a/pushy/src/test/java/com/relayrides/pushy/apns/util/ApnsPayloadBuilderTest.java b/pushy/src/test/java/com/relayrides/pushy/apns/util/ApnsPayloadBuilderTest.java
index f7791486..05ffc52c 100644
--- a/pushy/src/test/java/com/relayrides/pushy/apns/util/ApnsPayloadBuilderTest.java
+++ b/pushy/src/test/java/com/relayrides/pushy/apns/util/ApnsPayloadBuilderTest.java
@@ -21,10 +21,13 @@
 
 package com.relayrides.pushy.apns.util;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
 
 import java.lang.reflect.Type;
-import java.nio.charset.Charset;
+import java.nio.charset.CharsetEncoder;
+import java.nio.charset.StandardCharsets;
 import java.util.List;
 import java.util.Map;
 
@@ -32,6 +35,7 @@ import org.junit.Before;
 import org.junit.Test;
 
 import com.google.gson.Gson;
+import com.google.gson.GsonBuilder;
 import com.google.gson.reflect.TypeToken;
 
 public class ApnsPayloadBuilderTest {
@@ -43,7 +47,7 @@ public class ApnsPayloadBuilderTest {
 
     @Before
     public void setUp() {
-        this.gson = new Gson();
+        this.gson = new GsonBuilder().serializeNulls().disableHtmlEscaping().create();
         this.builder = new ApnsPayloadBuilder();
     }
 
@@ -295,8 +299,9 @@ public class ApnsPayloadBuilderTest {
 
     @Test
     public void testBuildWithMaximumLength() {
-        final String reallyLongAlertMessage =
-                "All non-glanded recruited mercenaries now engaging in training excercises are herefore and forever ordered to desist. Aforementioned activities have resulted in cost-defective damage to training areas.";
+        final String reallyLongAlertMessage = "All non-glanded recruited mercenaries now engaging in training " +
+                "excercises are herefore and forever ordered to desist. Aforementioned activities have resulted in " +
+                "cost-defective damage to training areas.";
 
         final int maxLength = 128;
 
@@ -304,7 +309,8 @@ public class ApnsPayloadBuilderTest {
 
         final String payloadString = this.builder.buildWithMaximumLength(maxLength);
 
-        assertTrue(payloadString.getBytes(Charset.forName("UTF-8")).length <= maxLength);
+        assertTrue(reallyLongAlertMessage.getBytes(StandardCharsets.UTF_8).length > maxLength);
+        assertTrue(payloadString.getBytes(StandardCharsets.UTF_8).length == maxLength);
     }
 
     @Test
@@ -328,7 +334,34 @@ public class ApnsPayloadBuilderTest {
 
         final String payloadString = this.builder.buildWithMaximumLength(maxLength);
 
-        assertTrue(payloadString.getBytes(Charset.forName("UTF-8")).length <= maxLength);
+        assertTrue(payloadString.getBytes(StandardCharsets.UTF_8).length <= maxLength);
+    }
+
+    @Test
+    public void testGetSizeOfJsonEscapedUtf8Character() {
+        final CharsetEncoder encoder = StandardCharsets.UTF_8.newEncoder();
+
+        for (int codePoint = Character.MIN_CODE_POINT; codePoint < Character.MAX_CODE_POINT; codePoint++) {
+            if (encoder.canEncode((char) codePoint)) {
+                // We subtract 2 here for the quotes that will appear on either end of a JSON string
+                assertEquals("Escaped/encoded lengths should match for code point " + codePoint,
+                        this.gson.toJson(String.valueOf((char) codePoint)).getBytes(StandardCharsets.UTF_8).length - 2,
+                        ApnsPayloadBuilder.getSizeOfJsonEscapedUtf8Character((char) codePoint));
+            }
+        }
+    }
+
+    @Test
+    public void testGetLengthOfJsonEscapedUtf8StringFittingSize() {
+        {
+            final String stringThatFits = "Test!";
+
+            assertEquals(stringThatFits.length(),
+                    ApnsPayloadBuilder.getLengthOfJsonEscapedUtf8StringFittingSize(stringThatFits, Integer.MAX_VALUE));
+        }
+
+        assertEquals(4, ApnsPayloadBuilder.getLengthOfJsonEscapedUtf8StringFittingSize("This string is too long.", 4));
+        assertEquals(2, ApnsPayloadBuilder.getLengthOfJsonEscapedUtf8StringFittingSize("\n\tThis string has escaped characters.", 4));
     }
 
     @SuppressWarnings("unchecked")