server : add handle_slot_type lambda #11535
Conversation
This commit adds a lambda function by extracting the common code for saving and restoring slots. The motivation for this change is to reduce code duplication between the current two lambda expressions/functions.
examples/server/server.cpp
Outdated
| } | ||
|
|
||
| if (type == SERVER_TASK_TYPE_SLOT_SAVE) { | ||
| GGML_ASSERT(dynamic_cast<server_task_result_slot_save_load*>(result.get()) != nullptr); |
There was a problem hiding this comment.
missing check for SERVER_TASK_TYPE_SLOT_RESTORE
There was a problem hiding this comment.
btw I'm not sure if we can use template here to avoid having a long list of if..else
There was a problem hiding this comment.
I'll take a look at this 👍
There was a problem hiding this comment.
I've added a suggestion for this in 6cc2956.
This contain an addition of a field, server_task_type, and a virtual function in server_task_result. The idea being that the dynamic cast like this can be avoided:
GGML_ASSERT(dynamic_cast<server_task_result_slot_save_load*>(result.get()) != nullptr);In favor of checks like this:
GGML_ASSERT(result.get() != nullptr);
GGML_ASSERT(result.get()->get_server_task_type() == type);If this looks like alright then I can update the other places in server.cpp which also have similar dynamic_casts.
examples/server/server.cpp
Outdated
| handle_slot_type(req, res, id_slot, SERVER_TASK_TYPE_SLOT_RESTORE); | ||
| }; | ||
|
|
||
| const auto handle_slots_erase = [&ctx_server, &res_error, &res_ok](const httplib::Request & /* req */, httplib::Response & res, int id_slot) { |
There was a problem hiding this comment.
IMO we should also extend the function to be reused by handle_slots_erase. The only thing need to adapt is that SERVER_TASK_TYPE_SLOT_ERASE ignores filename
There was a problem hiding this comment.
I'll take a look at this too.
Rename handle_slot_type to handle_slot_impl.
This commit moves the code for handling slot erase to the handle_slot_impl function. This is done to avoid code duplication and to make the code more readable.
This commit adds a server_task_type field to the server_task_result
struct. This field is used to identify the type of the server task.
The motivation for adding this is that it might allow us to avoid using
dynamic_cast's when checking the type of the server_task_result.
For example, this could then be replaced with checks like this:
```c++
GGML_ASSERT(result.get() != nullptr);
GGML_ASSERT(result.get()->get_server_task_type() == type);
```
Add missing override keyword.
Remove break statement from switch case SERVER_TASK_TYPE_NONE.
|
I have cancelled the server workflow since it has been running for 2 hours and seems to be stuck. |
Sorry about that, I'll look into this. |
Add a check for the server_task_type in the server.cpp file. This is to ensure that the server_task_type is correct when the server_task_type is SERVER_TASK_TYPE_SLOT_RESTORE. This is because the server_task_type is SERVER_TASK_TYPE_SLOT_SAVE when the server_task_type is SERVER_TASK_TYPE_SLOT_RESTORE.
|
I had to rerun one of the server test due to a 503 from tinyllamas/stories260K.gguf. I ran into this locally earlier today as well. |
|
Closing as this is change is not worth the effort involved (mine or any reviewers). |
3 participants
This commit adds a lambda function by extracting the common code for saving and restoring slots.
The motivation for this change is to reduce code duplication between the current two lambda expressions/functions.