From 6a19cf5205cec8d9794b85cdb2175ecd56493ff4 Mon Sep 17 00:00:00 2001 From: cjihrig Date: Wed, 24 Oct 2018 11:50:45 -0400 Subject: [PATCH] grpc-js-core: use Map for metadata store In more recent versions of Node, Maps are more performant than POJOs when used as maps. Switching to Maps also eliminates an expensive delete operation, as well as uses of hasOwnProperty(). --- packages/grpc-js-core/src/metadata.ts | 47 +++++++++++---------- packages/grpc-js-core/test/test-metadata.ts | 23 +++++----- 2 files changed, 38 insertions(+), 32 deletions(-) diff --git a/packages/grpc-js-core/src/metadata.ts b/packages/grpc-js-core/src/metadata.ts index 05c1e4ec..df3203fb 100644 --- a/packages/grpc-js-core/src/metadata.ts +++ b/packages/grpc-js-core/src/metadata.ts @@ -4,19 +4,21 @@ const LEGAL_KEY_REGEX = /^[0-9a-z_.-]+$/; const LEGAL_NON_BINARY_VALUE_REGEX = /^[ -~]*$/; export type MetadataValue = string|Buffer; - -export interface MetadataObject { [key: string]: MetadataValue[]; } +export type MetadataObject = Map; function cloneMetadataObject(repr: MetadataObject): MetadataObject { - const result: MetadataObject = {}; - forOwn(repr, (value, key) => { - result[key] = value.map(v => { + const result: MetadataObject = new Map(); + + repr.forEach((value, key) => { + const clonedValue: MetadataValue[] = value.map(v => { if (v instanceof Buffer) { return Buffer.from(v); } else { return v; } }); + + result.set(key, clonedValue); }); return result; } @@ -64,7 +66,7 @@ function validate(key: string, value?: MetadataValue): void { * A class for storing metadata. Keys are normalized to lowercase ASCII. */ export class Metadata { - protected internalRepr: MetadataObject = {}; + protected internalRepr: MetadataObject = new Map(); /** * Sets the given value for the given key by replacing any other values @@ -76,7 +78,7 @@ export class Metadata { set(key: string, value: MetadataValue): void { key = normalizeKey(key); validate(key, value); - this.internalRepr[key] = [value]; + this.internalRepr.set(key, [value]); } /** @@ -89,10 +91,13 @@ export class Metadata { add(key: string, value: MetadataValue): void { key = normalizeKey(key); validate(key, value); - if (!this.internalRepr[key]) { - this.internalRepr[key] = [value]; + + const existingValue: MetadataValue[]|undefined = this.internalRepr.get(key); + + if (existingValue === undefined) { + this.internalRepr.set(key, [value]); } else { - this.internalRepr[key].push(value); + existingValue.push(value); } } @@ -103,9 +108,7 @@ export class Metadata { remove(key: string): void { key = normalizeKey(key); validate(key); - if (Object.prototype.hasOwnProperty.call(this.internalRepr, key)) { - delete this.internalRepr[key]; - } + this.internalRepr.delete(key); } /** @@ -116,11 +119,7 @@ export class Metadata { get(key: string): MetadataValue[] { key = normalizeKey(key); validate(key); - if (Object.prototype.hasOwnProperty.call(this.internalRepr, key)) { - return this.internalRepr[key]; - } else { - return []; - } + return this.internalRepr.get(key) || []; } /** @@ -130,7 +129,8 @@ export class Metadata { */ getMap(): {[key: string]: MetadataValue} { const result: {[key: string]: MetadataValue} = {}; - forOwn(this.internalRepr, (values, key) => { + + this.internalRepr.forEach((values, key) => { if (values.length > 0) { const v = values[0]; result[key] = v instanceof Buffer ? v.slice() : v; @@ -157,8 +157,11 @@ export class Metadata { * @param other A Metadata object. */ merge(other: Metadata): void { - forOwn(other.internalRepr, (values, key) => { - this.internalRepr[key] = (this.internalRepr[key] || []).concat(values); + other.internalRepr.forEach((values, key) => { + const mergedValue: MetadataValue[] = + (this.internalRepr.get(key) || []).concat(values); + + this.internalRepr.set(key, mergedValue); }); } @@ -168,7 +171,7 @@ export class Metadata { toHttp2Headers(): http2.OutgoingHttpHeaders { // NOTE: Node <8.9 formats http2 headers incorrectly. const result: http2.OutgoingHttpHeaders = {}; - forOwn(this.internalRepr, (values, key) => { + this.internalRepr.forEach((values, key) => { // We assume that the user's interaction with this object is limited to // through its public API (i.e. keys and values are already validated). result[key] = values.map((value) => { diff --git a/packages/grpc-js-core/test/test-metadata.ts b/packages/grpc-js-core/test/test-metadata.ts index d60bd214..f5e4d80c 100644 --- a/packages/grpc-js-core/test/test-metadata.ts +++ b/packages/grpc-js-core/test/test-metadata.ts @@ -1,7 +1,7 @@ import * as assert from 'assert'; import * as http2 from 'http2'; import {range} from 'lodash'; -import {Metadata} from '../src/metadata'; +import {Metadata, MetadataObject, MetadataValue} from '../src/metadata'; class TestMetadata extends Metadata { getInternalRepresentation() { @@ -277,21 +277,24 @@ describe('Metadata', () => { }; const metadataFromHeaders = TestMetadata.fromHttp2Headers(headers); const internalRepr = metadataFromHeaders.getInternalRepresentation(); - assert.deepEqual(internalRepr, { - key1: ['value1'], - key2: ['value2'], - key3: ['value3a', 'value3b'], - 'key-bin': [ - Buffer.from(range(0, 16)), Buffer.from(range(16, 32)), - Buffer.from(range(0, 32)) + const expected: MetadataObject = new Map([ + ['key1', ['value1']], ['key2', ['value2']], + ['key3', ['value3a', 'value3b']], + [ + 'key-bin', + [ + Buffer.from(range(0, 16)), Buffer.from(range(16, 32)), + Buffer.from(range(0, 32)) + ] ] - }); + ]); + assert.deepEqual(internalRepr, expected); }); it('creates an empty Metadata object from empty headers', () => { const metadataFromHeaders = TestMetadata.fromHttp2Headers({}); const internalRepr = metadataFromHeaders.getInternalRepresentation(); - assert.deepEqual(internalRepr, {}); + assert.deepEqual(internalRepr, new Map()); }); }); });