Skip to content

Commit

Permalink
Fix serializedobject append for excessively large bytes length
Browse files Browse the repository at this point in the history
  • Loading branch information
wltsmrz committed May 15, 2015
1 parent e66978f commit 3fa6bc9
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 11 deletions.
17 changes: 11 additions & 6 deletions src/serializedobject.js
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,18 @@ SerializedObject.prototype.append = function(bytes) {
bytes = bytes.buffer;
}

// Make sure both buffer and bytes are Array. Either could potentially be a
// Buffer.
// Make sure both buffer and bytes are Array. Either could be a Buffer.
if (Array.isArray(this.buffer) && Array.isArray(bytes)) {
// Array::concat is horribly slow where buffer length is 100 kbytes + One
// transaction with 1100 affected nodes took around 23 seconds to convert
// from json to bytes.
Array.prototype.push.apply(this.buffer, bytes);
// `this.buffer = this.buffer.concat(bytes)` can be unbearably slow for
// large bytes length and acceptable bytes length is limited for
// `Array.prototype.push.apply(this.buffer, bytes)` as every element in the
// bytes array is pushed onto the stack, potentially causing a RangeError
// exception. Both of these solutions are known to be problematic for
// ledger 7501326. KISS instead

for (var i = 0; i < bytes.length; i++) {
this.buffer.push(bytes[i]);
}
} else {
this.buffer = this.buffer.concat(bytes);
}
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/ledger-full-7501326.json

Large diffs are not rendered by default.

14 changes: 9 additions & 5 deletions test/ledger-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ var Ledger = require('ripple-lib').Ledger;
* @param ledger_index {Number}
* Expects a corresponding ledger dump in $repo/test/fixtures/ folder
*/
create_ledger_test = function (ledger_index) {
create_ledger_test = function (ledger_index, no_accounts) {
describe(String(ledger_index), function() {

var path = __dirname + '/fixtures/ledger-full-'+ledger_index+'.json';
Expand All @@ -16,10 +16,12 @@ create_ledger_test = function (ledger_index) {
ledger_json = JSON.parse(ledger_raw),
ledger = Ledger.from_json(ledger_json);

it('has account_hash of '+ ledger_json.account_hash, function() {
assert.equal(ledger_json.account_hash,
ledger.calc_account_hash({sanity_test:true}).to_hex());
})
if (!no_accounts) {
it('has account_hash of '+ ledger_json.account_hash, function() {
assert.equal(ledger_json.account_hash,
ledger.calc_account_hash({sanity_test:true}).to_hex());
})
}
it('has transaction_hash of '+ ledger_json.transaction_hash, function() {
assert.equal(ledger_json.transaction_hash,
ledger.calc_tx_hash().to_hex());
Expand All @@ -32,6 +34,8 @@ describe('Ledger', function() {
create_ledger_test(38129);
// Because, why not.
create_ledger_test(40000);
// 1311 AffectedNodes, no accounts
create_ledger_test(7501326, true);

describe('#calcAccountRootEntryHash', function () {
it('will calculate the AccountRoot entry hash for rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh', function () {
Expand Down

0 comments on commit 3fa6bc9

Please sign in to comment.