-
Notifications
You must be signed in to change notification settings - Fork 14
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
Issue 4 proper handling of k 1 encoding #5
Issue 4 proper handling of k 1 encoding #5
Conversation
Signed-off-by: Maxime Lubin <maxime.lubin@scality.com>
Signed-off-by: Maxime Lubin <maxime.lubin@scality.com>
Encoding the single parity of k+1 code is XORing all data parts with the existing, potentially random and unitilized content of the parity buffer. Do a copy first, then XOR all the rest. Signed-off-by: Maxime Lubin <maxime.lubin@scality.com>
dot_cpy(shards[i], target, shardSize); | ||
copied = 1; | ||
} else { | ||
dot_xor(shards[i], target, shardSize); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you please update the indentation before dot_xor()
to be the same as the indentation before dot_cpy()
?
Indentation should use 2 spaces, no tabs.
@@ -317,7 +317,7 @@ queue.onData = function(args, end) { | |||
); | |||
var buffer = cipher.update(Buffer.alloc(bufferOffset + bufferSize)); | |||
Assert(buffer.length === bufferOffset + bufferSize); | |||
var parity = Buffer.alloc(parityOffset + paritySize); | |||
var parity = Node.crypto.randomBytes(parityOffset + paritySize); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please would you use this instead:
var parity = cipher.update(Buffer.alloc(parityOffset + paritySize));
The AES cipher is faster than a call to crypto.randomBytes()
, saving around 2 seconds (10%) of total runtime, for the same guarantees.
@@ -360,10 +360,6 @@ queue.onData = function(args, end) { | |||
for (var i = 0; i < k; i++) Assert(Hash(shards[i]) === hashes[i]); | |||
for (var i = k; i < k + m; i++) hashes[i] = Hash(shards[i]); | |||
// Test against fixed vector: | |||
var key = ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for removing this artifact.
@@ -764,6 +760,6 @@ queue.concat([ | |||
[24, 5, 13, 9, 8, '4968f6d39f355c41e006b754b436d823'], | |||
[24, 5, 4, 12, 89816, '93e82c3652f9f963eed72e0d640d9528'], | |||
[24, 6, 10, 5, 8, '1cd9f45259767305dd7b565bcf9c0359'], | |||
[24, 6, 3, 1, 164320, 'f2fad94fdb9e4518728cb0a5a5dbe392'] | |||
[24, 6, 3, 1, 164320, 'f2fad94fdb9e4518728cb0a5a5dbe392'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you please remove the trailing comma that was added here?
Thanks @maximelubin , I made the requested PR changes already in another PR: |
Encoding the single parity of k+1 code is XORing all data parts with the existing, potentially random and uninitialized content of the parity buffer.
Do a copy first, then XOR all the rest.