waitinfuture commented on PR #2119: URL: https://github.com/apache/incubator-celeborn/pull/2119#issuecomment-1845588778
Hi @xleoken , thanks for this PR! In my personal opinion, performance precedes readability in most cases (if we can't achieve both), though I know it's controversial. Even though this piece of code is not in perf-critical path, I think we shouldn't replace it with lower performance implementation. In fact the readability of original code is natural to me, it copies two segments of sublists, instead of copying one by one. And the original implementation is more perf robust against different list types. Adding comments could be better. BTW, other modification of this PR looks good to me :) -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
