Optimize some iterations in BodyExtractor and BodyInserter#30136
Optimize some iterations in BodyExtractor and BodyInserter#30136simonbasle merged 2 commits intospring-projects:mainfrom
Conversation
they use less memory and cpu
simonbasle
left a comment
There was a problem hiding this comment.
indeed these kind of stream usage can get an order of magnitude slower especially for small collections... this looks good to me overall, thanks for the PR @yuzawa-san 👍
one minor comment to address from a readability standpoint and I think we can merge this in time for 6.0.8
| .orElseGet(() -> Mono.error(unsupportedError(bodyType, context, mediaType))); | ||
| for (HttpMessageWriter<?> messageWriter : context.messageWriters()) { | ||
| if (messageWriter.canWrite(bodyType, mediaType)) { | ||
| return write(publisher, bodyType, mediaType, outputMessage, context, cast(messageWriter)); |
There was a problem hiding this comment.
compared to the BodyExtractor case, I find that cast call is a little too buried among all these parameters, making it less visible. would you mind extracting it to an intermediate local variable? this should be efficiently optimized away by the JIT compiler, so that is purely from a readability perspective.
use an intermediate variable for readability
|
@simonbasle updated with intermediate variable. #29972 also gets around another (I would say more expensive) streams usage in ReadOnlyHttpHeaders. I left this one there because that PR avoids that entrySet and also I have been working on a branch where I got ReadOnlyHttpHeaders to use CollectionUtils.unmodifiableMultiValueMap internally.
|
simonbasle
left a comment
There was a problem hiding this comment.
thanks for the change @yuzawa-san 👍
Using iterators for trivial findFirst and toList cases uses less CPU and memory (in these cases, not while costing readability)
Here is an icicle graph example of the CPU usage before
and here it is after:
There was about a 10-15% overhead with the streams usage, which we can save on by doing this.