[core] Avoid resize in GetAndPinArgsForExecutor#51543
[core] Avoid resize in GetAndPinArgsForExecutor#51543edoakes merged 3 commits intoray-project:masterfrom
Conversation
Signed-off-by: dayshah <dhyey2019@gmail.com>
| arg_refs->at(i) = arg_ref; | ||
| by_ref_indices[arg_id].push_back(i); | ||
| arg_refs->push_back(arg_ref); | ||
| args->emplace_back(); |
There was a problem hiding this comment.
Curious why do we emplace here?
So in the old impl, args[i] is assigned once via
args->at(i) = std::make_shared<RayObject>(data, metadata, task.ArgInlinedRefs(i), copy_data);in the new impl, it's emplaced twice
args->emplace_back(); // first time
args->push_back(std::make_shared<RayObject>(std::move(data), std::move(metadata), task.ArgInlinedRefs(i), copy_data)); // second timeThere was a problem hiding this comment.
it's actually if else, so only once like
if something:
args->emplace_back(); // first time
else:
args->push_back(std::make_shared<RayObject>(std::move(data), std::move(metadata), task.ArgInlinedRefs(i), copy_data)); // second time
before since we did resize before we'd have the empty shared ptr there anyways in the if case
There was a problem hiding this comment.
oh icic, sorry for the misread.
There was a problem hiding this comment.
btw is it possible to early exit?
https://clang.llvm.org/extra/clang-tidy/checks/readability/else-after-return.html
it's actually a good place to use, the if-else block is so big
There was a problem hiding this comment.
ya it is possible here but i'd rather not for this because we can save the git blame here and it doesn't really add much to readability in this case
Resize is pretty inefficient default constructing all these in the case of a lot of args. --------- Signed-off-by: dayshah <dhyey2019@gmail.com>
Resize is pretty inefficient default constructing all these in the case of a lot of args. --------- Signed-off-by: dayshah <dhyey2019@gmail.com> Signed-off-by: Dhakshin Suriakannu <d_suriakannu@apple.com>
Resize is pretty inefficient default constructing all these in the case of a lot of args. --------- Signed-off-by: dayshah <dhyey2019@gmail.com> Signed-off-by: Srinath Krishnamachari <srinath.krishnamachari@anyscale.com>
4 participants
Why are these changes needed?
Resize is pretty inefficient default constructing all these in the case of a lot of args.
Related issue number
Checks
git commit -s) in this PR.scripts/format.shto lint the changes in this PR.method in Tune, I've added it in
doc/source/tune/api/under thecorresponding
.rstfile.