diff --git a/bench/index.js b/bench/index.js index 7a8a55fbe..dfe260b65 100644 --- a/bench/index.js +++ b/bench/index.js @@ -10,7 +10,8 @@ // in between. var newSuite = require("./suite"), - payload = require("./data/bench.json"); + payload = require("./data/bench.json"), + protobuf = require(".."); var Buffer_from = Buffer.from !== Uint8Array.from && Buffer.from || function(value, encoding) { return new Buffer(value, encoding); }; @@ -88,3 +89,40 @@ newSuite("combined") jspbCls.deserializeBinary(jspbMsg.serializeBinary()); }) .run(); + +var json = require("../tests/data/test.json"); +newSuite("fromJSON") +.add("isolated", function() { + protobuf.Root.fromJSON(json); +}) +.add("isolated (resolveAll)", function() { + protobuf.Root.fromJSON(json).resolveAll(); +}) +.add("shared (unique)", function() { + var root = protobuf.Root.fromJSON(json); + for (var i = 0; i < 1000; ++i) { + var jsonCopy = { + options: json.options, + nested: {} + }; + // eslint-disable-next-line no-loop-func + Object.keys(json).forEach(key => { + jsonCopy.nested[key + i] = json[key]; + }); + + protobuf.Root.fromJSON(jsonCopy, root); + } +}).run(); + +var resolveAllRoot = protobuf.Root.fromJSON(json); +newSuite("resolveAll") +.add("isolated", function() { + resolveAllRoot.resolveAll(); +}).run(); + +newSuite("load") +.add("sync", function() { + protobuf.loadSync("bench/data/bench.proto"); +}) +.run(); + diff --git a/index.d.ts b/index.d.ts index 6e211631d..fb96caec1 100644 --- a/index.d.ts +++ b/index.d.ts @@ -750,6 +750,9 @@ export abstract class NamespaceBase extends ReflectionObject { /** Nested objects by name. */ public nested?: { [k: string]: ReflectionObject }; + /** Whether or not objects contained in this namespace need feature resolution. */ + protected _needsRecursiveFeatureResolution: boolean; + /** Nested objects of this namespace as an array for iteration. */ public readonly nestedArray: ReflectionObject[]; @@ -909,6 +912,9 @@ export abstract class ReflectionObject { /** Resolved Features. */ public _features: object; + /** Whether or not features have been resolved. */ + public _featuresResolved: boolean; + /** Parent namespace. */ public parent: (Namespace|null); diff --git a/package.json b/package.json index 717b5f105..df40083fe 100644 --- a/package.json +++ b/package.json @@ -33,7 +33,9 @@ "build:bundle": "gulp --gulpfile scripts/gulpfile.js", "build:types": "node cli/bin/pbts --main --global protobuf --out index.d.ts src/ lib/aspromise/index.js lib/base64/index.js lib/codegen/index.js lib/eventemitter/index.js lib/float/index.js lib/fetch/index.js lib/inquire/index.js lib/path/index.js lib/pool/index.js lib/utf8/index.js", "changelog": "node scripts/changelog -w", - "coverage": "nyc tape -r ./lib/tape-adapter tests/*.js tests/node/*.js", + "coverage": "npm run coverage:test && npm run coverage:report", + "coverage:test": "nyc --silent tape -r ./lib/tape-adapter tests/*.js tests/node/*.js", + "coverage:report": "nyc report --reporter=lcov --reporter=text", "docs": "jsdoc -c config/jsdoc.json -R README.md --verbose --pedantic", "lint": "npm run lint:sources && npm run lint:types", "lint:sources": "eslint \"**/*.js\" -c config/eslint.json", diff --git a/src/field.js b/src/field.js index b71969e2e..72d8f63e0 100644 --- a/src/field.js +++ b/src/field.js @@ -370,12 +370,18 @@ Field.prototype._inferLegacyProtoFeatures = function _inferLegacyProtoFeatures(e } var features = {}; - this.resolve(); + if (this.rule === "required") { features.field_presence = "LEGACY_REQUIRED"; } - if (this.resolvedType instanceof Type && this.resolvedType.group) { - features.message_encoding = "DELIMITED"; + if (this.parent && types.defaults[this.type] === undefined) { + // We can't use resolvedType because types may not have been resolved yet. However, + // legacy groups are always in the same scope as the field so we don't have to do a + // full scan of the tree. + var type = this.parent.get(this.type.split(".").pop()); + if (type && type instanceof Type && type.group) { + features.message_encoding = "DELIMITED"; + } } if (this.getOption("packed") === true) { features.repeated_field_encoding = "PACKED"; diff --git a/src/namespace.js b/src/namespace.js index 51ba2bd24..2a79f296a 100644 --- a/src/namespace.js +++ b/src/namespace.js @@ -108,10 +108,33 @@ function Namespace(name, options) { * @private */ this._nestedArray = null; + + /** + * Cache lookup calls for any objects contains anywhere under this namespace. + * This drastically speeds up resolve for large cross-linked protos where the same + * types are looked up repeatedly. + * @type {Object.} + * @private + */ + this._lookupCache = {}; + + /** + * Whether or not objects contained in this namespace need feature resolution. + * @type {boolean} + * @protected + */ + this._needsRecursiveFeatureResolution = true; } function clearCache(namespace) { namespace._nestedArray = null; + namespace._lookupCache = {}; + + // Also clear parent caches, since they include nested lookups. + var parent = namespace; + while(parent = parent.parent) { + parent._lookupCache = {}; + } return namespace; } @@ -249,6 +272,14 @@ Namespace.prototype.add = function add(object) { } } + this._needsRecursiveFeatureResolution = true; + + // Also clear parent caches, since they need to recurse down. + var parent = this; + while(parent = parent.parent) { + parent._needsRecursiveFeatureResolution = true; + } + object.onAdd(this); return clearCache(this); }; @@ -324,6 +355,9 @@ Namespace.prototype.resolveAll = function resolveAll() { * @override */ Namespace.prototype._resolveFeaturesRecursive = function _resolveFeaturesRecursive(edition) { + if (!this._needsRecursiveFeatureResolution) return this; + this._needsRecursiveFeatureResolution = false; + edition = this._edition || edition; ReflectionObject.prototype._resolveFeaturesRecursive.call(this, edition); @@ -341,7 +375,6 @@ Namespace.prototype._resolveFeaturesRecursive = function _resolveFeaturesRecursi * @returns {ReflectionObject|null} Looked up object or `null` if none could be found */ Namespace.prototype.lookup = function lookup(path, filterTypes, parentAlreadyChecked) { - /* istanbul ignore next */ if (typeof filterTypes === "boolean") { parentAlreadyChecked = filterTypes; @@ -360,25 +393,48 @@ Namespace.prototype.lookup = function lookup(path, filterTypes, parentAlreadyChe if (path[0] === "") return this.root.lookup(path.slice(1), filterTypes); + var found = this._lookupImpl(path); + if (found && (!filterTypes || filterTypes.indexOf(found.constructor) > -1)) { + return found; + } + + // If there hasn't been a match, try again at the parent + if (this.parent === null || parentAlreadyChecked) + return null; + return this.parent.lookup(path, filterTypes); +}; + +/** + * Internal helper for lookup that handles searching just at this namespace and below along with caching. + * @param {string[]} path Path to look up + * @returns {ReflectionObject|null} Looked up object or `null` if none could be found + * @private + */ +Namespace.prototype._lookupImpl = function lookup(path) { + var flatPath = path.join("."); + if(Object.prototype.hasOwnProperty.call(this._lookupCache, flatPath)) { + return this._lookupCache[flatPath]; + } + // Test if the first part matches any nested object, and if so, traverse if path contains more var found = this.get(path[0]); + var exact = null; if (found) { if (path.length === 1) { - if (!filterTypes || filterTypes.indexOf(found.constructor) > -1) - return found; - } else if (found instanceof Namespace && (found = found.lookup(path.slice(1), filterTypes, true))) - return found; + exact = found; + } else if (found instanceof Namespace && (found = found._lookupImpl(path.slice(1)))) + exact = found; // Otherwise try each nested namespace - } else + } else { for (var i = 0; i < this.nestedArray.length; ++i) - if (this._nestedArray[i] instanceof Namespace && (found = this._nestedArray[i].lookup(path, filterTypes, true))) - return found; + if (this._nestedArray[i] instanceof Namespace && (found = this._nestedArray[i]._lookupImpl(path))) + exact = found; + } - // If there hasn't been a match, try again at the parent - if (this.parent === null || parentAlreadyChecked) - return null; - return this.parent.lookup(path, filterTypes); + // Set this even when null, so that when we walk up the tree we can quickly bail on repeated checks back down. + this._lookupCache[flatPath] = exact; + return exact; }; /** diff --git a/src/object.js b/src/object.js index bf367e267..0ea6a2334 100644 --- a/src/object.js +++ b/src/object.js @@ -67,6 +67,12 @@ function ReflectionObject(name, options) { */ this._features = {}; + /** + * Whether or not features have been resolved. + * @type {boolean} + */ + this._featuresResolved = false; + /** * Parent namespace. * @type {Namespace|null} @@ -172,10 +178,8 @@ ReflectionObject.prototype.onRemove = function onRemove(parent) { ReflectionObject.prototype.resolve = function resolve() { if (this.resolved) return this; - if (this instanceof Root) { - this._resolveFeaturesRecursive(this._edition); - this.resolved = true; - } + if (this.root instanceof Root) + this.resolved = true; // only if part of a root return this; }; @@ -194,6 +198,10 @@ ReflectionObject.prototype._resolveFeaturesRecursive = function _resolveFeatures * @returns {undefined} */ ReflectionObject.prototype._resolveFeatures = function _resolveFeatures(edition) { + if (this._featuresResolved) { + return; + } + var defaults = {}; /* istanbul ignore if */ @@ -217,6 +225,7 @@ ReflectionObject.prototype._resolveFeatures = function _resolveFeatures(edition) throw new Error("Unknown edition: " + edition); } this._features = Object.assign(defaults, protoFeatures || {}); + this._featuresResolved = true; return; } @@ -238,6 +247,7 @@ ReflectionObject.prototype._resolveFeatures = function _resolveFeatures(edition) // Sister fields should have the same features as their extensions. this.extensionField._features = this._features; } + this._featuresResolved = true; }; /** diff --git a/src/root.js b/src/root.js index 8e3c72c4a..fa6e10be0 100644 --- a/src/root.js +++ b/src/root.js @@ -51,7 +51,7 @@ Root.fromJSON = function fromJSON(json, root) { root = new Root(); if (json.options) root.setOptions(json.options); - return root.addJSON(json.nested).resolveAll(); + return root.addJSON(json.nested)._resolveFeaturesRecursive(); }; /** @@ -99,6 +99,9 @@ Root.prototype.load = function load(filename, options, callback) { // Finishes loading by calling the callback (exactly once) function finish(err, root) { + if (root) { + root._resolveFeaturesRecursive(); + } /* istanbul ignore if */ if (!callback) { return; @@ -108,9 +111,6 @@ Root.prototype.load = function load(filename, options, callback) { } var cb = callback; callback = null; - if (root) { - root.resolveAll(); - } cb(err, root); } @@ -218,8 +218,8 @@ Root.prototype.load = function load(filename, options, callback) { for (var i = 0, resolved; i < filename.length; ++i) if (resolved = self.resolvePath("", filename[i])) fetch(resolved); - self.resolveAll(); if (sync) { + self._resolveFeaturesRecursive(); return self; } if (!queued) { @@ -272,6 +272,7 @@ Root.prototype.resolveAll = function resolveAll() { throw Error("unresolvable extensions: " + this.deferred.map(function(field) { return "'extend " + field.extend + "' in " + field.parent.fullName; }).join(", ")); + this._resolveFeaturesRecursive(this._edition); return Namespace.prototype.resolveAll.call(this); }; diff --git a/src/service.js b/src/service.js index 2c11ec9e3..a7be2f483 100644 --- a/src/service.js +++ b/src/service.js @@ -121,6 +121,8 @@ Service.prototype.resolveAll = function resolveAll() { * @override */ Service.prototype._resolveFeaturesRecursive = function _resolveFeaturesRecursive(edition) { + if (!this._needsRecursiveFeatureResolution) return this; + edition = this._edition || edition; Namespace.prototype._resolveFeaturesRecursive.call(this, edition); diff --git a/src/type.js b/src/type.js index 40c9f7868..c11fc6d65 100644 --- a/src/type.js +++ b/src/type.js @@ -317,6 +317,8 @@ Type.prototype.resolveAll = function resolveAll() { * @override */ Type.prototype._resolveFeaturesRecursive = function _resolveFeaturesRecursive(edition) { + if (!this._needsRecursiveFeatureResolution) return this; + edition = this._edition || edition; Namespace.prototype._resolveFeaturesRecursive.call(this, edition);