Performance improvement, typically 25%, by splitting internal 'is' function

[lovell]

Hello, at my current place of work we’ve been processing relatively large XML documents with expat but recently started trialling the use of the object-building xml2js, which depends upon this module.

Performance was of concern, so I carried out some V8 profiling and discovered a few hot spots:

 [C++]:
   ticks  total  nonlib   name
    121    5.0%    5.0%  int v8::internal::BinarySearch<(v8::internal::SearchMode)1, v8::internal::DescriptorArray>(v8::internal::DescriptorArray*, v8::internal::Name*, int, int*)
     98    4.0%    4.1%  v8::internal::Object::ObjectProtoToString(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>)

 [C++ entry points]:
   ticks    cpp   total   name
    566   41.0%   23.2%  v8::internal::Builtin_ObjectProtoToString(int, v8::internal::Object**, v8::internal::Isolate*)
    178   12.9%    7.3%  v8::internal::Runtime_StringEqual(int, v8::internal::Object**, v8::internal::Isolate*)
    122    8.8%    5.0%  v8::internal::Runtime_KeyedLoadIC_Miss(int, v8::internal::Object**, v8::internal::Isolate*)

This module currently has an internal is function that accepts either a RegExp or an Object. Much of the time is spent determining what data type has been passed to it by calling Object.prototype.toString.

Luckily the type is already known at all the call sites for this function, so this PR splits the is function into an Object version and a RegExp version. The RegExp version also uses the short-circuiting regex.test rather than a full regex.match.

Profiling with the changes in this PR reveals the hot spots are gone:

[C++]:
   ticks  total  nonlib   name
     63    4.0%    4.0%  node::ContextifyScript::New(v8::FunctionCallbackInfo<v8::Value> const&)

[C++ entry points]:
   ticks    cpp   total   name
     83   12.3%    5.2%  v8::internal::Runtime_KeyedLoadIC_Miss(int, v8::internal::Object**, v8::internal::Isolate*)

Performance testing with real world XML of various orders of magnitude with a Node v6 runtime suggests performance gains are in the 25-40% region, possibly slightly more for even larger documents or longer running processes.

XML size/MB	Current time/ms	New time/ms	Speed-up
0.1	61	45	25%
1	323	210	35%
10	2550	1535	40%

The contribution guide asks for tests but I'm unsure of the best way to further test this change beyond the existing and already-extensive unit test suite; guidance welcome here.

Thank you for continuing to maintain this highly-depended upon module.

[isaacs]

Landed on master and pushed to npm. Thanks!

[lovell]

It looks like I underestimated the possible performance gains this change can bring, especially for very short XML strings.

By way of example, the node-expat 25 byte benchmark demonstrates a 2x improvement.

sax v1.2.1:

sax x 60,837 ops/sec ±1.54% (83 runs sampled)
node-expat x 440,272 ops/sec ±2.04% (83 runs sampled)

sax v1.2.2:

sax x 133,001 ops/sec ±1.59% (78 runs sampled)
node-expat x 440,120 ops/sec ±1.43% (85 runs sampled)

...so thank you for merging!