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

[C++] Introduce userver #7617

Merged
merged 17 commits into from
Nov 27, 2022
Merged

[C++] Introduce userver #7617

merged 17 commits into from
Nov 27, 2022

Conversation

itrofimow
Copy link
Contributor

@itrofimow itrofimow commented Oct 2, 2022

This PR aims to incorporate https://github.com/userver-framework/userver into benchmark suite

@itrofimow itrofimow changed the title Userver [C++] Introduce userver Oct 2, 2022
@itrofimow
Copy link
Contributor Author

@apolukhin @userver-framework can you guys please have a look at this?

There def. are some configurations tweaks to make, but i'm not yet certain which ones exactly - guess it can be done after merging this PR and seeing benchmark results.

@apolukhin
Copy link

A few nitpicks for later changes:

  • Please write userver in lower case
  • You could replace userver::formats::json::ValueBuilder with formats::json::StringBuilder to slightly speedup the service
  • Adjusting ev and worker threads count would improve the results. Experiment qith those valies to find the best ones.
  • If target architecture is know, use -march= and -mtune= compiler flags to save a few more CPU cycles

@itrofimow
Copy link
Contributor Author

A few nitpicks for later changes:

  • Please write userver in lower case
  • You could replace userver::formats::json::ValueBuilder with formats::json::StringBuilder to slightly speedup the service
  • Adjusting ev and worker threads count would improve the results. Experiment qith those valies to find the best ones.
  • If target architecture is know, use -march= and -mtune= compiler flags to save a few more CPU cycles

Fixed userver spelling, added march=native.

  • About formats::json::StringBuilder - quoting TFB docs, "All test implementations should be production-grade", and i'm not sure how production-grade that is (but feel free to disagree and fix it later).

  • Adjusting ev and worker threads count: that definitely should be done, but i feel like it's best changed with some results at hand

@itrofimow
Copy link
Contributor Author

@nbrady-techempower Hi! Could you please have a look at this?

@apolukhin
Copy link

About formats::json::StringBuilder - quoting TFB docs, "All test implementations should be production-grade", and i'm not sure how production-grade that is (but feel free to disagree and fix it later).

We use it quite a lot in production for serializing responses. It's an interface for streaming serializer (SAX), which is a quite a common technique. There's also a separate section in docs with a sample for SAX streaming https://userver.tech/db/dd7/md_en_userver_formats.html#streamingserialization . However, it may not provide notable advantage for small responses.

userver::formats::json::Type::kObject};
builder["id"] = row.id;
builder["randomNumber"] = row.random_number;
return builder.ExtractValue();
Copy link

Choose a reason for hiding this comment

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

an inline builder (formats::json::MakeDoc) should perform better here

@itrofimow
Copy link
Contributor Author

Not sure what went wrong here, runs fine in my action queue: itrofimow#1

@nbrady-techempower could you please rerun failing checks for me?

@itrofimow
Copy link
Contributor Author

Hi @nbrady-techempower!
Thank you for rerunning the checks.

Is there anything holding this PR back from being merged?

@NateBrady23 NateBrady23 merged commit e20a158 into TechEmpower:master Nov 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants