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

execution time for encodeAsBinary #60

Closed
WaqqasMeraj opened this issue Jan 4, 2017 · 6 comments
Closed

execution time for encodeAsBinary #60

WaqqasMeraj opened this issue Jan 4, 2017 · 6 comments

Comments

@WaqqasMeraj
Copy link

Our profiling reveals that encodeAsBinary takes almost double the time as compared to encodeAsString for same data. Any ideas why is that and what can we do or what assumptions can we take to reduce this time?
Note: we use protobuf to convert our JSONs to binary.

@darrachequesne
Copy link
Member

encodeAsBinary takes almost double the time as compared to encodeAsString for same data

What do you mean by "same data"? encodeAsBinary should handle messages that contains binary, and encodeAsString messages that doesn't. Or have I misunderstood?

@WaqqasMeraj
Copy link
Author

By same data, I meant the JSON in raw form given to encodeAsString is encoded twice faster than that same JSON converted to binary and then given to encodeAsBinary. so, eg:

jsonPack = { id: 'json', type: 2, data: { code: 404, msg: 'Resource not found' } }
when given to Encoder.encode() takes on around average: 9.91 micro seconds

while

binPack = { id: 'bin', type: 5, data: <Buffer 08 94 03 12 12 52 65 73 6f 75 72 63 65 20 6e 6f 74 20 66 6f 75 6e 64> }

where binPack.data is infact protobuffed binary of jsonPack.data, takes about 20.05 micro seconds.

Let me know if more information is required.

@darrachequesne
Copy link
Member

darrachequesne commented Jan 6, 2017

var parser = require('socket.io-parser');
var encoder = new parser.Encoder();

var packet = {
  id: 'json',
  type: 2,
  data: { code: 404, msg: 'Resource not found' }
};

var binaryPacket = {
  id: 'bin',
  type: 5,
  data: new Buffer('\x08\x94\x03\x12\x12\x52\x65\x73\x6f\x75\x72\x63\x65\x20\x6e\x6f\x74\x20\x66\x6f\x75\x6e\x64', 'binary')
};

encoder.encode(packet, function(encodedPackets) {
  console.log('packet => ', encodedPackets);
  // packet =>  [ '2json{"code":404,"msg":"Resource not found"}' ]
});

encoder.encode(binaryPacket, function(encodedPackets) {
  console.log('binaryPacket => ', encodedPackets);
  // binaryPacket =>  [ '51-bin{"_placeholder":true,"num":0}',
  // <Buffer 08 94 03 12 12 52 65 73 6f 75 72 63 65 20 6e 6f 74 20 66 6f 75 6e 64> ]
});

Currently, a packet containing binary data gets encoded as (at least) two packets, the first one being the "structure" of the packet without binary elements, and the other(s) being those binary elements. So there are a few extra steps with binary data, that should explain the difference in computing time.

Now, could it be faster? That's a good question.

@WaqqasMeraj
Copy link
Author

WaqqasMeraj commented Jan 6, 2017

Given the guarantees:

  1. data to be encoded will never contain Blob or File, and
  2. will always be in binary format exclusively (that is, it will never be JSON + binary). So, deconstructPacket will always result in 1 attachment (if I have understood the code correctly.).

can we make it faster then?

eg: we could skip invocation of removeBlobs if its safe. Anything else that you could recommend given these guarantees or refer to any other binary encoding?

@darrachequesne
Copy link
Member

I'm not sure about what you're suggesting. Something like function encodeAsBinary(obj, skipBlobCheck, callback)?

Yet I'm not sure if the reward would be really worth the change.

@WaqqasMeraj
Copy link
Author

Let me get back to you with a POC and compute times.

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

No branches or pull requests

2 participants