jnturton commented on code in PR #2800:
URL: https://github.com/apache/drill/pull/2800#discussion_r1189557009


##########
exec/java-exec/src/main/java/org/apache/drill/exec/store/http/oauth/OAuthUtils.java:
##########
@@ -36,6 +37,7 @@
 
 public class OAuthUtils {
   private static final Logger logger = 
LoggerFactory.getLogger(OAuthUtils.class);
+  private static final ObjectMapper MAPPER = JacksonUtils.createObjectMapper();

Review Comment:
   We're converting method scope ObjectMappers to static class members which is 
efficient in terms of rework but will add a bit to Drill's fixed heap 
requirement since they can never be collected. It looks like ObjectMappers are 
thread safe so, for the cases where the caller does not need to do mapper 
customisation, could we get even better reuse from a new singleton 
`JacksonUtils.DEFAULT_MAPPER`?



##########
common/src/test/java/org/apache/drill/test/DrillTest.java:
##########
@@ -37,11 +38,10 @@
 
 public class DrillTest extends BaseTest {
 
-  protected static final ObjectMapper objectMapper;
+  private static final ObjectMapper objectMapper = 
JacksonUtils.createObjectMapper();

Review Comment:
   Nice catch, thank you.



-- 
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: dev-unsubscr...@drill.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to