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

WIP: RESP3 #34

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

WIP: RESP3 #34

wants to merge 1 commit into from

Conversation

EALePain
Copy link
Contributor

I'm actively working on a RESP 3 implementation for fun and figured I'd share it for comment before I get too deep into it. I am trying to maintain an interface as close as possible to your RESP 2 implementation, although I've had to make a few compromises to make the recursiveness work without use of boost::variant (although from what I've read, boost variant would have involved more heap allocations than is strictly necessary).

I had a couple specific concerns I want to bring up:

  • I went with the first idea I had for creating the map/attributes, set, and push implementations: a buffer managed entirely within the methods, with those methods defined after the declaration of array. I checked using compiler explorer that sizeof(std::map<...>) etc doesn't change based on the data type, but it still feels a bit hacky to do this. Are there any better ideas that don't involve an extra heap allocation? Or do you think that extra heap allocation is fine?
  • I tend to struggle a bit with organizing my Metaprogramming tools and external, but still header, method definitions; if you have any suggestions here that would be greatly appreciated.
  • I hand rolled a copyable pointer to minimize the impact of having to support the fact attributes can be on literally any element. I couldn't find any but are there any value_ptr like objects I could grab from boost (sense its already a dependency) so it doesn't have to be maintained by this repo?
  • I am somewhat unsatisfied with the name I went with for the double: double_float if you have better suggestions I am 100% willing to consider them.
  • I noticed while testing that the attributes marker is actually described to be ` and | on the RESP documentation page at different locations, and I can't seem to do a full search of the redis interface documentation to find a message that is responded to with data that has attributes, do you know a way I might be able to confirm which is actually used?
  • When I was writing the get_marker functions I did not realize how close it was in name to get_mark. But I can't quite think of a name that is sufficiently different.

I am not quite ready to call this ready for merge, I'd like to add resp3 to serialization side and get some tests written for both. But any and all comments would be appreciated.

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.

1 participant