feat: support batch embedding requests.#1828
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces support for batch text embedding requests by adding handle_text_embedding_request in api_service.cpp and implementing BatchEmbeddingContext and process_batch_async in embedding_service_impl.cpp. Feedback highlights two critical issues: first, a potential crash when calling contains() on a JSON body that is not an object, which can be resolved by checking is_object() first; second, duplicate request_ids in batch requests that could cause scheduler collisions, which should be resolved by appending the batch index to each request ID.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| std::vector<std::string> inputs; | ||
| bool is_batch = false; | ||
| if (json_body.contains("input")) { |
There was a problem hiding this comment.
Calling contains() on a nlohmann::json object when it is not of object type (e.g., if the client sends a JSON array, null, or primitive value) will throw a nlohmann::json::type_error exception, causing the request thread to crash or abort. Check json_body.is_object() first to ensure safe access.
| if (json_body.contains("input")) { | |
| if (json_body.is_object() && json_body.contains("input")) { |
| int64_t created_time = absl::ToUnixSeconds(absl::Now()); | ||
|
|
||
| size_t num_requests = inputs.size(); | ||
| std::vector<RequestParams> sps(num_requests, request_params); |
There was a problem hiding this comment.
Copying request_params directly for all requests in the batch results in duplicate request_ids. In the engine's scheduler, request_id must be unique to prevent collisions, state overwrites, or crashes. Appending the batch index to the base request_id ensures uniqueness while maintaining traceability. This also adheres to the style guide by using reserve() and emplace_back().
std::vector<RequestParams> sps;
sps.reserve(num_requests);
for (size_t i = 0; i < num_requests; ++i) {
RequestParams sp = request_params;
sp.request_id = request_id + "-" + std::to_string(i);
sps.emplace_back(std::move(sp));
}References
- Always reserve() before filling a std::vector when the size is known or can be estimated, and prefer emplace_back over push_back. (link)
3387347 to
1fc3a47
Compare
1fc3a47 to
fb46f81
Compare
179ce60 to
f3bc98b
Compare
Description
Related Issues
Change Type
Pull Request Checklist
Thank you for contributing to xLLM. Before requesting review, please make sure the following items are complete.
PR Title and Commit Messages
<type>: <subject>.Pre-commit Checks
pre-commitby runningpip install pre-commitor an equivalent command.pre-commit install.pre-commit run --all-filesand fixed any reported issues.Self Review
.agents/skills/code-review/references/custom-code-style.md, especially code written or assisted by AI.mainbranch.Build and Test Coverage
python setup.py build testhas passed on a CUDA machine.python setup.py build testhas passed on an NPU machine.python setup.py build testhas passed on an MLU machine.Reviewer Notes