Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(server): add latency statistics for lua script calls #758

Merged
merged 1 commit into from
Feb 5, 2023

Conversation

romange
Copy link
Collaborator

@romange romange commented Feb 5, 2023

Also provide new subcommands "script list" and "script latency" Fixes #754

Signed-off-by: Roman Gershman [email protected]

@romange romange added the enhancement New feature or request label Feb 5, 2023
@romange romange requested a review from dranikpg February 5, 2023 11:35
@romange romange force-pushed the MeasureEvalLatencyt branch from c86a766 to 3fbd436 Compare February 5, 2023 12:00
dranikpg
dranikpg previously approved these changes Feb 5, 2023
Copy link
Contributor

@dranikpg dranikpg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You missed a few moves here and there when passing the scripts around 👀 , but I assume its not important at all

@romange
Copy link
Collaborator Author

romange commented Feb 5, 2023 via email

Comment on lines 457 to 462
auto scripts = sf_->script_mgr()->GetLuaScripts();
ec = saver->SaveHeader(scripts);
StringVec script_bodies;
for (const auto& script : scripts) {
script_bodies.push_back(script.second);
}
ec = saver->SaveHeader(script_bodies);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You get the scripts by value here and proceed to build a new container by copying the scripts itself

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Comment on lines 880 to 885
auto scripts = script_mgr_->GetLuaScripts();
StringVec script_bodies;
for (const auto& script : scripts) {
script_bodies.push_back(script.second);
}
return script_bodies;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Also provide new subcommands "script list" and "script latency"
Fixes #754

Signed-off-by: Roman Gershman <[email protected]>
@romange romange merged commit 4c725b6 into main Feb 5, 2023
@romange romange deleted the MeasureEvalLatencyt branch February 5, 2023 19:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

allow listing lua scripts
2 participants