refactor: optimize code #504
Conversation
| const toComparators = (range, options) => | ||
| new Range(range, options).set | ||
| .map(comp => comp.map(c => c.value).join(' ').trim().split(' ')) | ||
| .map(comp => comp.map(c => c.value.trim())) |
There was a problem hiding this comment.
this isn't exactly the same thing - [' a ', ' b '] would have previously produced ['a ', ' b'] but will now produce ['a', 'b'].
|
|
||
| // normalize spaces | ||
| range = range.split(/\s+/).join(' ') | ||
| range = range.replace(/ +/g, ' ') |
There was a problem hiding this comment.
this change means it will no longer select non-space whitespace characters, including tabs and other unicode whitespace chars.
There was a problem hiding this comment.
you should provide tests to protect it
There was a problem hiding this comment.
Indeed, there should be tests, but few projects are willing to change behavior just because there’s not a test yet.
|
There are several similar fragments in the code - they basically solve the task assigned to them, but from the point of view of performance, this is just the worst option - we temporarily create 2 arrays and 2 rows and then throw out their garbage. For execution in the browser - this is a window when it comes to working in a server environment where thousands of requests are executed in parallel - a useless load is created for the garbage collector, which negatively affects the performance of the application |
lukekarrys
left a comment
lukekarrys
left a comment
There was a problem hiding this comment.
@budarin can you provide benchmarks showing the performance difference of these changes?
|
There are no benchmarks, but according to the experience of developing services at high loads, the generation of thousands of requests for such garbage turns into jumps in performance - first a pile of such garbage grows, then the process is suspended by the garbage collector and then everything repeats again Here, even benchmarks are not needed to understand that extra garbage (generating multiple arrays and strings) just to get another string from the source string will lead to memory costs and, accordingly, increase the time for garbage collection |
|
Consider storing versions internally not in a string representation https://marvinh.dev/blog/speeding-up-javascript-ecosystem/ |
|
Closing in favor of the discussion/improvements in #528 |
4 participants
Replace suboptimal code with efficient one