This repository was archived by the owner on Oct 30, 2023. It is now read-only.
forked from joaopsilva/binance-java-api
-
Notifications
You must be signed in to change notification settings - Fork 607
This repository was archived by the owner on Oct 30, 2023. It is now read-only.
Optimization: Jackson ObjectMapper instance shouldn't be constructed on each WebSocket Message #29
Copy link
Copy link
Closed
Description
Related code:
binance-java-api/src/main/java/com/binance/api/client/impl/BinanceApiWebSocketListener.java
Lines 26 to 35 in 74289e4
| @Override | |
| public void onMessage(WebSocket webSocket, String text) { | |
| ObjectMapper mapper = new ObjectMapper(); | |
| try { | |
| T event = mapper.readValue(text, eventClass); | |
| callback.onResponse(event); | |
| } catch (IOException e) { | |
| throw new BinanceApiException(e); | |
| } | |
| } |
ObjectMapper is fairly expensive to create, and especially for Android, this can create quite the heavy pressure on the GC and memory usage, considering it is created on every socket message.
Therefore I kindly suggest to either:
- Delegate the instance construction to the
BinanceApiWebSocketListenerclass constructor so we can allow more flexibility in creatingObjectMapper. - And maybe create the
ObjectMapperinstance if it wasn't given and hold it locally as a private reference in a secondary constructor. This can maintain the current library compatibility.
The same issue also present in UserDataUpdateEventDeserializer
Lines 45 to 52 in 74289e4
| public <T> T getUserDataUpdateEventDetail(String json, Class<T> clazz) { | |
| ObjectMapper mapper = new ObjectMapper(); | |
| try { | |
| return mapper.readValue(json, clazz); | |
| } catch (IOException e) { | |
| throw new BinanceApiException(e); | |
| } | |
| } |
I can help creating a pull request later to fix this, but if you guys have any other idea or other implementations in mind, then please feel free to share.
Metadata
Metadata
Assignees
Labels
No labels