Optimize some iterations in BodyExtractor and BodyInserter by yuzawa-san · Pull Request #30136 · spring-projects/spring-framework (original) (raw)
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.
they use less memory and cpu
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)); |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
| @Override |
|---|
| public Set<Entry<String, List<String>>> entrySet() { |
| return this.headers.entrySet().stream().map(SimpleImmutableEntry::new) |
| .collect(Collectors.collectingAndThen( |
| Collectors.toCollection(LinkedHashSet::new), // Retain original ordering of entries |
| Collections::unmodifiableSet)); |
| } |
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use iterators in BodyExtractor and BodyInserter Optimize some iterations in BodyExtractor and BodyInserter
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})