[FABN-1342] Improve SDK packaging performance (#47)

Use custom writable stream implementation instead of inefficient stream-buffers package when building smart contract packages. Signed-off-by: Simon Stone <[email protected]> Change-Id: I51de2aa6cc62933340dd31e6b3af36f376c6f646
hyperledger · Nov 29, 2019 · 9e9ec9e · 9e9ec9e
1 parent cf63bcc
commit 9e9ec9e
Show file tree

Hide file tree

Showing 10 changed files with 92 additions and 53 deletions.
diff --git a/fabric-client/lib/packager/BufferStream.js b/fabric-client/lib/packager/BufferStream.js
@@ -0,0 +1,27 @@
+/*
+ * Copyright 2019 IBM All Rights Reserved.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+'use strict';
+
+const stream = require('stream');
+
+class BufferStream extends stream.PassThrough {
+
+	constructor() {
+		super();
+		this.buffers = [];
+		this.on('data', (chunk) => {
+			this.buffers.push(chunk);
+		});
+	}
+
+	toBuffer() {
+		return Buffer.concat(this.buffers);
+	}
+
+}
+
+module.exports = BufferStream;
diff --git a/fabric-client/lib/packager/Golang.js b/fabric-client/lib/packager/Golang.js
@@ -16,10 +16,10 @@
 const fs = require('fs-extra');
 const klaw = require('klaw');
 const path = require('path');
-const sbuf = require('stream-buffers');
 const {Utils: utils} = require('fabric-common');
 
 const BasePackager = require('./BasePackager');
+const BufferStream = require('./BufferStream');
 
 const logger = utils.getLogger('packager/Golang.js');
 
@@ -71,9 +71,9 @@ class GolangPackager extends BasePackager {
 			descriptors = srcDescriptors.concat(metaDescriptors);
 		}
 
-		const buffer = new sbuf.WritableStreamBuffer();
-		await super.generateTarGz(descriptors, buffer);
-		return buffer.getContents();
+		const stream = new BufferStream();
+		await super.generateTarGz(descriptors, stream);
+		return stream.toBuffer();
 	}
 
 	/**

diff --git a/fabric-client/lib/packager/Java.js b/fabric-client/lib/packager/Java.js
@@ -15,14 +15,14 @@
 'use strict';
 
 const path = require('path');
-const sbuf = require('stream-buffers');
 const {Utils: utils} = require('fabric-common');
 
 const walk = require('ignore-walk');
 
 const logger = utils.getLogger('JavaPackager.js');
 
 const BasePackager = require('./BasePackager');
+const BufferStream = require('./BufferStream');
 
 class JavaPackager extends BasePackager {
 
@@ -35,17 +35,16 @@ class JavaPackager extends BasePackager {
 	async package(chaincodePath, metadataPath) {
 		logger.debug(`packaging Java source from ${chaincodePath}`);
 
-		const buffer = new sbuf.WritableStreamBuffer();
 		let descriptors = await this.findSource(chaincodePath);
 		if (metadataPath) {
 			logger.debug(`packaging metadata files from ${metadataPath}`);
 
 			const metaDescriptors = await super.findMetadataDescriptors(metadataPath);
 			descriptors = descriptors.concat(metaDescriptors);
 		}
-		await super.generateTarGz(descriptors, buffer);
-
-		return buffer.getContents();
+		const stream = new BufferStream();
+		await super.generateTarGz(descriptors, stream);
+		return stream.toBuffer();
 	}
 
 	/**

diff --git a/fabric-client/lib/packager/Node.js b/fabric-client/lib/packager/Node.js
@@ -15,14 +15,14 @@
 'use strict';
 
 const path = require('path');
-const sbuf = require('stream-buffers');
 const {Utils: utils} = require('fabric-common');
 
 const walk = require('ignore-walk');
 
 const logger = utils.getLogger('packager/Node.js');
 
 const BasePackager = require('./BasePackager');
+const BufferStream = require('./BufferStream');
 
 class NodePackager extends BasePackager {
 
@@ -44,15 +44,15 @@ class NodePackager extends BasePackager {
 		// strictly necessary yet, they pave the way for the future where we
 		// will need to assemble sources from multiple packages
 
-		const buffer = new sbuf.WritableStreamBuffer();
 		const srcDescriptors = await this.findSource(projDir);
 		let descriptors = srcDescriptors;
 		if (metadataPath) {
 			const metaDescriptors = await super.findMetadataDescriptors(metadataPath);
 			descriptors = srcDescriptors.concat(metaDescriptors);
 		}
-		await super.generateTarGz(descriptors, buffer);
-		return buffer.getContents();
+		const stream = new BufferStream();
+		await super.generateTarGz(descriptors, stream);
+		return stream.toBuffer();
 	}
 
 	/**

diff --git a/fabric-client/package.json b/fabric-client/package.json
@@ -33,7 +33,6 @@
     "klaw": "^2.0.0",
     "long": "^4.0.0",
     "promise-settle": "^0.3.0",
-    "stream-buffers": "3.0.1",
     "tar-stream": "1.6.1",
     "url": "^0.11.0",
     "yn": "^3.1.0"

diff --git a/fabric-client/test/packager/BufferStream.js b/fabric-client/test/packager/BufferStream.js
@@ -0,0 +1,43 @@
+/*
+ * Copyright 2019 IBM All Rights Reserved.
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+'use strict';
+
+const BufferStream = require('../../lib/packager/BufferStream');
+require('chai').should();
+
+describe('BufferStream', () => {
+
+	describe('#toBuffer', () => {
+
+		it('should return an empty buffer when no buffers written', () => {
+			const bufferStream = new BufferStream();
+			bufferStream.toBuffer().should.have.lengthOf(0);
+		});
+
+		it('should return the correct buffer when one buffer is written', () => {
+			const bufferStream = new BufferStream();
+			bufferStream.write(Buffer.from('hello world'));
+			const buffer = bufferStream.toBuffer();
+			buffer.should.have.lengthOf(11);
+			buffer.toString().should.equal('hello world');
+		});
+
+		it('should return the correct buffer when multiple buffers are written', () => {
+			const bufferStream = new BufferStream();
+			bufferStream.write(Buffer.from('hello'));
+			bufferStream.write(Buffer.from(' '));
+			bufferStream.write(Buffer.from('world'));
+			bufferStream.write(Buffer.from(' from '));
+			bufferStream.write(Buffer.from('multiple buffers'));
+			const buffer = bufferStream.toBuffer();
+			buffer.should.have.lengthOf(33);
+			buffer.toString().should.equal('hello world from multiple buffers');
+		});
+
+	});
+
+});
diff --git a/fabric-client/test/packager/Golang.js b/fabric-client/test/packager/Golang.js
@@ -15,6 +15,7 @@
 'use strict';
 
 const rewire = require('rewire');
+const BufferStream = require('../../lib/packager/BufferStream');
 const chai = require('chai');
 const chaiAsPromised = require('chai-as-promised');
 chai.use(chaiAsPromised);
@@ -28,8 +29,6 @@ describe('Golang', () => {
 	let FakeLogger;
 	let findMetadataDescriptorsStub;
 	let generateTarGzStub;
-	let bufferStub;
-	let getContentsStub;
 
 	let golang;
 	beforeEach(() => {
@@ -45,13 +44,6 @@ describe('Golang', () => {
 		generateTarGzStub = sandbox.stub().resolves();
 		revert.push(Golang.__set__('BasePackager.prototype.findMetadataDescriptors', findMetadataDescriptorsStub));
 		revert.push(Golang.__set__('BasePackager.prototype.generateTarGz', generateTarGzStub));
-		getContentsStub = sandbox.stub();
-		bufferStub = class {
-			constructor() {
-				this.getContents = getContentsStub;
-			}
-		};
-		revert.push(Golang.__set__('sbuf.WritableStreamBuffer', bufferStub));
 
 		golang = new Golang();
 	});
@@ -67,15 +59,13 @@ describe('Golang', () => {
 		it('should return the package when given the metadata path', async() => {
 			findSourceStub.resolves(['descriptor2']);
 			await golang.package('ccpath', 'metadatapath');
-			sinon.assert.calledWith(generateTarGzStub, ['descriptor2', 'descriptor1'], new bufferStub());
-			sinon.assert.called(getContentsStub);
+			sinon.assert.calledWith(generateTarGzStub, ['descriptor2', 'descriptor1'], sinon.match.instanceOf(BufferStream));
 		});
 
 		it('should return the package when not given the metadata path', async() => {
 			findSourceStub.resolves(['descriptor2']);
 			await golang.package('ccpath');
-			sinon.assert.calledWith(generateTarGzStub, ['descriptor2'], new bufferStub());
-			sinon.assert.called(getContentsStub);
+			sinon.assert.calledWith(generateTarGzStub, ['descriptor2'], sinon.match.instanceOf(BufferStream));
 		});
 	});
 

diff --git a/fabric-client/test/packager/Java.js b/fabric-client/test/packager/Java.js
@@ -15,6 +15,7 @@
 'use strict';
 
 const rewire = require('rewire');
+const BufferStream = require('../../lib/packager/BufferStream');
 const chai = require('chai');
 const chaiAsPromised = require('chai-as-promised');
 chai.use(chaiAsPromised);
@@ -28,8 +29,6 @@ describe('Java', () => {
 	let FakeLogger;
 	let findMetadataDescriptorsStub;
 	let generateTarGzStub;
-	let bufferStub;
-	let getContentsStub;
 
 	let java;
 	beforeEach(() => {
@@ -47,13 +46,6 @@ describe('Java', () => {
 		generateTarGzStub = sandbox.stub().resolves();
 		revert.push(Java.__set__('BasePackager.prototype.findMetadataDescriptors', findMetadataDescriptorsStub));
 		revert.push(Java.__set__('BasePackager.prototype.generateTarGz', generateTarGzStub));
-		getContentsStub = sandbox.stub();
-		bufferStub = class {
-			constructor() {
-				this.getContents = getContentsStub;
-			}
-		};
-		revert.push(Java.__set__('sbuf.WritableStreamBuffer', bufferStub));
 
 		java = new Java();
 	});
@@ -69,15 +61,13 @@ describe('Java', () => {
 		it('should return the package when given the metadata path', async () => {
 			findSourceStub.resolves(['descriptor2']);
 			await java.package('ccpath', 'metadatapath');
-			sinon.assert.calledWith(generateTarGzStub, ['descriptor2', 'descriptor1'], new bufferStub());
-			sinon.assert.called(getContentsStub);
+			sinon.assert.calledWith(generateTarGzStub, ['descriptor2', 'descriptor1'], sinon.match.instanceOf(BufferStream));
 		});
 
 		it('should return the package when not given the metadata path', async () => {
 			findSourceStub.resolves(['descriptor2']);
 			await java.package('ccpath');
-			sinon.assert.calledWith(generateTarGzStub, ['descriptor2'], new bufferStub());
-			sinon.assert.called(getContentsStub);
+			sinon.assert.calledWith(generateTarGzStub, ['descriptor2'], sinon.match.instanceOf(BufferStream));
 		});
 	});
 

diff --git a/fabric-client/test/packager/Node.js b/fabric-client/test/packager/Node.js
@@ -16,6 +16,7 @@
 'use strict';
 
 const rewire = require('rewire');
+const BufferStream = require('../../lib/packager/BufferStream');
 const chai = require('chai');
 const chaiAsPromised = require('chai-as-promised');
 chai.use(chaiAsPromised);
@@ -29,8 +30,6 @@ describe('Node', () => {
 	let FakeLogger;
 	let findMetadataDescriptorsStub;
 	let generateTarGzStub;
-	let bufferStub;
-	let getContentsStub;
 
 	let node;
 	beforeEach(() => {
@@ -46,13 +45,6 @@ describe('Node', () => {
 		generateTarGzStub = sandbox.stub().resolves();
 		revert.push(Node.__set__('BasePackager.prototype.findMetadataDescriptors', findMetadataDescriptorsStub));
 		revert.push(Node.__set__('BasePackager.prototype.generateTarGz', generateTarGzStub));
-		getContentsStub = sandbox.stub();
-		bufferStub = class {
-			constructor() {
-				this.getContents = getContentsStub;
-			}
-		};
-		revert.push(Node.__set__('sbuf.WritableStreamBuffer', bufferStub));
 
 		node = new Node();
 	});
@@ -68,15 +60,13 @@ describe('Node', () => {
 		it('should return the package when given the metadata path', async() => {
 			findSourceStub.resolves(['descriptor2']);
 			await node.package('ccpath', 'metadatapath');
-			sinon.assert.calledWith(generateTarGzStub, ['descriptor2', 'descriptor1'], new bufferStub());
-			sinon.assert.called(getContentsStub);
+			sinon.assert.calledWith(generateTarGzStub, ['descriptor2', 'descriptor1'], sinon.match.instanceOf(BufferStream));
 		});
 
 		it('should return the package when not given the metadata path', async() => {
 			findSourceStub.resolves(['descriptor2']);
 			await node.package('ccpath');
-			sinon.assert.calledWith(generateTarGzStub, ['descriptor2'], new bufferStub());
-			sinon.assert.called(getContentsStub);
+			sinon.assert.calledWith(generateTarGzStub, ['descriptor2'], sinon.match.instanceOf(BufferStream));
 		});
 	});
 

diff --git a/package.json b/package.json
@@ -84,6 +84,7 @@
     "run-sequence": "^2.2.1",
     "sinon": "6.1.3",
     "sinon-chai": "^3.3.0",
+    "stream-buffers": "3.0.1",
     "strip-ansi": "5.2.0",
     "tap-colorize": "^1.2.0",
     "tape": "^4.5.1",