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

Concat -> simple push #370

Merged
merged 1 commit into from
Jun 7, 2021
Merged

Concat -> simple push #370

merged 1 commit into from
Jun 7, 2021

Conversation

maciejhirsz
Copy link
Contributor

Was looking into batching out of curiosity to see if we need to change our sinks from using String to Box<RawValue> (we don't) and just found this small thing that could be better.

@@ -44,8 +44,8 @@ pub async fn collect_batch_response(rx: mpsc::UnboundedReceiver<String>) -> Stri
let mut buf = String::with_capacity(2048);
buf.push('[');
let mut buf = rx
.fold(buf, |mut acc, response| async {
acc = [acc, response].concat();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I assumed this would alloc since Concat is implemented on Borrow<str>, and just checking ASM produced for this, it does indeed alloc on every call here, so the capacity isn't even being used.

Copy link
Contributor

@dvdplm dvdplm left a comment

Choose a reason for hiding this comment

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

Good catch, I wrongly assumed concat, being specially built for the purpose, would be faster.

@dvdplm dvdplm merged commit d5ba2bd into master Jun 7, 2021
@dvdplm dvdplm deleted the mh-concat2push branch June 7, 2021 08:51
dvdplm added a commit that referenced this pull request Jul 6, 2021
* master: (21 commits)
  New proc macro (#387)
  Streaming RpcParams parsing (#401)
  Set allowed Host header values (#399)
  Synchronization-less async connections in ws-server (#388)
  [ws server]: terminate already established connection(s) when the server is stopped (#396)
  feat: customizable JSON-RPC error codes via new enum variant on `CallErrror` (#394)
  [ci]: test each individual crate's manifest (#392)
  Add a way to stop servers (#386)
  [jsonrpsee types]: unify a couple of types + more tests (#389)
  Update roadmap link in readme (#390)
  Cross-origin protection (#375)
  Method aliases + RpcModule: Clone (#383)
  Use criterion's async bencher (#385)
  Async/subscription benches (#372)
  send text (#374)
  Fix link to ws server in README.md (#373)
  Concat -> simple push (#370)
  Add missing `rt` feature (#369)
  Release prep for v0.2 (#368)
  chore(scripts): publish script (#354)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants