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

Skip allocations, serialization and deserialization of request data for in-memory requests #6021

Closed

Conversation

nb-ohad
Copy link
Contributor

@nb-ohad nb-ohad commented May 17, 2020

Explain the changes

Add in-memory request type that holds the request data in memory
The new request type is used by in-process requests. to skip the buffer allocations, serialization, and deserialization of the request data and buffers.

Issues: Fixed #xxx / Gap #xxx

Testing Instructions:

…sing a WeakMap)

The new request type is used by in-process requests. to skip the buffer allocations,
serialization, and deserialization of the request data and buffers.
@nb-ohad nb-ohad requested a review from guymguym May 17, 2020 19:45
Copy link
Member

@guymguym guymguym left a comment

Choose a reason for hiding this comment

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

@nb-ohad I am not sure about all these changes in rpc.js.
I don't want all those functions that used to take a request and operate on it to change to avoid it as it just makes the code complex.
If you just want to control the encoding, maybe we can simply move the encode/decode functions from request to the connection? Would that work?

@nb-ohad
Copy link
Contributor Author

nb-ohad commented May 18, 2020

@guymguym
I don't see the problem, the request is the same (the new request derive from the last one) the only thing that changed is the internal representation of the data in the request.

I don't really want only to control the encoding/decoding I want to eliminate the allocation of a new set of buffers + serialization and deserialization.

RPC works just the way it worked before with a slight change where the class for requests is selected in run time based on the connection type.

We can move the encode and decode implementation into a diff place (like a message encoder/decoder) but this will not eliminate the need of a seperate request representation

Thoughts?

@guymguym
Copy link
Member

Closing - this PR was replaced by #6027

@guymguym guymguym closed this Jun 10, 2020
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.

2 participants