feat(refactor): refactor toESDocument() to be more efficient

[missinglink]

refactor code for performance & readability

this PR removes the slow JSON serialize/deserialize code. that code converts each object to a string and then back in to an object in order to take advantage of a feature in JSON.stringify which removes empty properties.

it turns out this is potentially slow, so I refactored the code to do the same thing using a different method.

benchmarks:

#### calling constructor, all setter methods and toESDocument()

# before (after the previous PR was merged)
imported 10000 docs in 1124 ms
average speed: 0.1124 ms

# after
imported 10000 docs in 290 ms
average speed: 0.0290 ms

works out to be ~75% faster, multiplied out to 500 million records, this change could save 10+ hrs

[orangejulius]

Holy 💩! This is huge

[trescube]

wouldn't it be faster to conditionally add these properties than add and then conditionally remove them?

[missinglink]

I didn't test it but I think doing something like:

if( !doc.hasOwnProperty( 'parent' ) ){ doc.parent = []; }

would be less efficient as it would need to be executed once per call to addParent()

[trescube]

No, I mean don't set parent when initializing doc, then doing

if (!_.isEmpty(this.parent) { 
  doc.parent = this.parent 
}

[trescube]

_.isEmpty is somewhat inefficient in this case because it checks for 5 other types of empty conditions before it finally gets to plain old objects. You might see better performance by just checking Object.keys(this.parent).length since you know it's always going to be an plain old object.

[orangejulius]

@trescube arguing AGAINST using lodash? you feelin ok bud?

[trescube]

I know! It pained me to type that.

[missinglink]

agh I see what you mean, to be honest I did it like this so that the key ordering of the output matched the fixtures, I suspect that if we change the key ordering then there will be a big cleanup effort with all the unit tests fixtures across all the importers.

I don't think the delete call is very expensive so I'm happy to pay that cost in order to avoid having to fix all the fixture files :)

[missinglink]

could do it like this? it's not so pretty :(

Document.prototype.toESDocument = function() {

  var doc = {
    name: this.name,
    phrase: this.phrase
  };

  if( !_.isEmpty( doc.parent ) ){ doc.parent = this.parent; }
  if( !_.isEmpty( doc.address_parts ) ){ doc.address_parts = this.address_parts; }
  doc.center_point = this.center_point;
  if( !_.isEmpty( doc.category ) ){ doc.category = this.category; }
  doc.category = this.category;
  doc.source = this.source;
  doc.layer = this.layer;
  doc.source_id = this.source_id;

  return {
    _index: config.schema.indexName,
    _type: this.getType(),
    _id: this.getId(),
    data: doc
  };
};

[orangejulius]

can you do the performance tests for that most recent version you linked as well? i've read (can't find the link now but will look for it) that modifying an object many times like this, rather than setting as many properties as possible when the object is first created, is bad for performance. it would be interesting to see what the actual difference (if any) is