redisparse api problems

redisparse_api_update.md

##redisparse api problems

redisparse: https://github.com/tonistiigi/redisparse/

optimizations branch: https://github.com/tonistiigi/redisparse/compare/make-it-fast

compatible node_redis: https://github.com/tonistiigi/node_redis/compare/redisparse-integration

benchmarks: https://gist.github.com/tonistiigi/4973004

Current

Same as node_redis javascript parser with added partial data emitter:

execute()
on('reply', ...)
on('reply error', ...)
on('error', ...)
on('reply partial', ...) <- new

Problems

• Works well for always streaming API(kv) but not perfect for mixing streaming and callbacks(node_redis).
• Client library has to handle concatenation of data. https://github.com/tonistiigi/node_redis/commit/5ebff2735e427e0ec72c58996d4f67fec277fbdc#L0R269
• This can be non-optimal. String_decoder can be avoided for callback methods. Also think avoiding extra slice() calls gives speed benefit.
• No way to control it from client library. (can't just make new parser instance for every command.).
• I don't get why event emitters are used here. They have no benefits and are quite slow.
• Not 100% drop-in replacement for node_redis/hiredis.

Solution 1

Make partial replies optional. Don't emit partial data but only emit intent that partial data is available. Then its up to client library to decide if it wants to use it or not.

parser.on('reply partial', function(p) {
  p.getData()
})

If nothing calls getData() then reply emits full concatenated response. If getData() is called then it only emits last part. 100% compatible with hiredis/node_redis.

Problems

• Weird API for doing a streaming client
• Still using unnecessary event emitters

Solution 2

Use hiredis API.

feed()
getResult()

getResult() can take in boolen parameter partial. If this is true parser may return partial result.

Problems

• Node_redis has to manage 2 different ways for calling parsers. One for hiredis and redisparse and other for current javascript parser(cause I don't want to rewrite that one). This is ~15 lines of code.

Solution 3

Use streams2 style read() and write(). Although this would be nice in theory I don't think it would be the fastest and would require even more code change in node_redis.