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

perf: bypass duplexify #29

Closed
wants to merge 2 commits into from
Closed

perf: bypass duplexify #29

wants to merge 2 commits into from

Conversation

ronag
Copy link
Contributor

@ronag ronag commented Mar 7, 2021

Bypasses duplexify and pipes encode, socket, decode directly without any intermediate proxy.

Bypasses duplexify and pipes encode, socket, decode directly
without any intermediate proxy.
@ronag
Copy link
Contributor Author

ronag commented Mar 7, 2021

Would be nice if multileveldown provided a less hacky way to do this...

index.js Outdated Show resolved Hide resolved
@vweevers
Copy link
Member

vweevers commented Mar 7, 2021

The goal here is to skip this line, if I understand it correctly? https://github.com/Level/multileveldown/blob/52f4861fc37500bfed167114f007bfce3104fc77/leveldown.js#L79.

@ronag
Copy link
Contributor Author

ronag commented Mar 7, 2021

The goal here is to skip this line, if I understand it correctly? https://github.com/Level/multileveldown/blob/52f4861fc37500bfed167114f007bfce3104fc77/leveldown.js#L79.

Yep and avoid the overhead and complexity of an intermediate stream.

@vweevers
Copy link
Member

vweevers commented Mar 7, 2021

The less hacky way would be to support a second argument here, defaulting to null:

https://github.com/Level/multileveldown/blob/52f4861fc37500bfed167114f007bfce3104fc77/client.js#L21-L23

@ronag
Copy link
Contributor Author

ronag commented Mar 7, 2021

The less hacky way would be to support a second argument here, defaulting to null:

Yep. But that would require updating multileveldown.

ronag added a commit to ronag/multileveldown that referenced this pull request Mar 7, 2021
@vweevers
Copy link
Member

vweevers commented Mar 7, 2021

I worry this will have a side effects, because:

  • duplexify handles a bunch of edge cases
  • the close event on a net socket has a hadError argument that we may need to ignore here

@ronag
Copy link
Contributor Author

ronag commented Mar 7, 2021

I worry this will have a side effects, because:

  • duplexify handles a bunch of edge cases

I'm more worried about having duplexify than not having it.

  • the close event on a net socket has a hadError argument that we may need to ignore here

Yes.

ronag added a commit to ronag/multileveldown that referenced this pull request Mar 7, 2021
vweevers pushed a commit to Level/multileveldown that referenced this pull request Mar 7, 2021
@vweevers
Copy link
Member

vweevers commented Mar 18, 2022

This makes no difference in my benchmarks, at least not after removing much bigger bottlenecks. In rave-level (the WIP replacement for level-party) I doubled the throughput by batching reads, among other things.

@vweevers vweevers closed this Mar 18, 2022
@vweevers vweevers closed this Mar 18, 2022
@ronag
Copy link
Contributor Author

ronag commented Mar 18, 2022

Can I follow many-level somewhere?

@vweevers
Copy link
Member

I'll post something in Level/abstract-level#14 once I've pushed the repo

vweevers added a commit to Level/many-level that referenced this pull request Mar 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants