RUM-6256: Optimize MD5 byte array to hex string conversion#2273
Conversation
a0ef0c2 to
99cd1bc
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2273 +/- ##
===========================================
+ Coverage 69.95% 70.00% +0.05%
===========================================
Files 731 732 +1
Lines 27182 27204 +22
Branches 4573 4576 +3
===========================================
+ Hits 19013 19043 +30
+ Misses 6885 6878 -7
+ Partials 1284 1283 -1
|
99cd1bc to
a8b9222
Compare
a8b9222 to
7512200
Compare
mariusc83
left a comment
mariusc83
left a comment
There was a problem hiding this comment.
Nice job overall just a small comment regarding the tests.
7512200 to
eca5f4d
Compare
| ExtendWith(ForgeExtension::class) | ||
| ) | ||
| @MockitoSettings(strictness = Strictness.LENIENT) | ||
| class ByteArrayExtTest { |
There was a problem hiding this comment.
Note
Since the goal of this is to optimize the performance of converting a bytearray to string, shouldn't we add a performance test ?
- Make a large byte array
- convert it to hex string using this new function, and joinToString
- compare the performance of both methods
There was a problem hiding this comment.
Interesting point, However I am not sure the performance test should be part of the unit test, since unit test is supposed to verify correctness. Also I am afraid of performance test can be flaky, it depends also on the running environment, the test can fail even though we know in general toHexString is better than joinToString.
So do you have any suggestion how we should proceed this performance test?
There was a problem hiding this comment.
I will merge it since I got enough approval, don't hesitate to add anything here, we can do it in another PR!
5 participants
What does this PR do?
This PR changes the way to convert MD5 bytes to HEX string.
Motivation
When we broke down the steps of generating image wireframe, we noticed that
joinToStringfunction takes a huge amount of time when generating the MD5 resource id:Suspicious reasons:
String Concatenation Overhead:
joinToString internally uses StringBuilder, but string concatenation inside the lambda (using
"%02x".format(it)) could be inefficient. If format() is invoked for each byte, it involves repeated function calls, which adds to overhead.Repeated Memory Allocations:
"%02x".format(it)creates a new string for each byte, which leads to multiple temporary objects being created. This puts pressure on the garbage collector.Outcome
After using the new function to convert the byte arrays, we can see the duration optimisation rate is up to 99%
Span list
Time series
Review checklist (to be filled by reviewers)