singhpk234 commented on code in PR #13004:
URL: https://github.com/apache/iceberg/pull/13004#discussion_r2135226197
##########
core/src/main/java/org/apache/iceberg/rest/HTTPClient.java:
##########
@@ -329,7 +341,8 @@ protected <T extends RESTResponse> T execute(
}
try {
- return mapper.readValue(responseBody, responseType);
+ ObjectReader reader =
mapper.readerFor(responseType).with(injectableValues);
Review Comment:
precisely, infact I went ahead and cached `mapper.readerFor(responseType)`
at our HTTPClient level to avoid any even slighest of recomputation in case
jackson misses part of caching, even though jackson caches `_typeCache`
so the flow is we create a reader only once per client, now when the
HTTPClient is called with a ParserContext, we create a clone of a reader which
is literally just change injectedValues, no re:computation for reader just use
the one we passed.
```java
/**
* Method for constructing a new instance with configuration that uses
* passed {@link InjectableValues} to provide injectable values.
*<p>
* Note that the method does NOT change state of this reader, but
* rather construct and returns a newly configured instance.
*/
public ObjectReader with(InjectableValues injectableValues)
{
if (_injectableValues == injectableValues) {
return this;
}
return _new(this, _config,
_valueType, _rootDeserializer, _valueToUpdate,
_schema, injectableValues, _dataFormatReaders);
}
```
It would have been ever better is the ObjectReader had some setter but i
think by design they are suppose to be immutable. We should be good both from
perf perspective and correctness perspective. waiting to hear from others !
sure thing, added a new class with wrapper over injectedValues.
--
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: issues-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
