Remove isNative

[conorcussell]

@ianstormtaylor I've removed isNative

I guess the library benchmarks don't give us much info on the performance changes here, any tips on checking the performance of this in the browser?

benchmarks
    models
      get-blocks
        295.26 → 283.60 iterations/sec
      get-blocks-at-range
        2.73 → 2.80 iterations/sec
      get-characters
        86.72 → 100.63 iterations/sec
      get-characters-at-range
        7.47 → 8.48 iterations/sec
      get-inlines
        294.12 → 294.65 iterations/sec
      get-inlines-at-range
        2.64 → 2.68 iterations/sec
      get-marks
        17.04 → 17.32 iterations/sec
      get-marks-at-range
        5.04 → 4.42 iterations/sec
      get-path
        37.46 → 38.18 iterations/sec
      get-ranges
        1098.85 → 1056.25 iterations/sec
      get-texts
        259.95 → 244.42 iterations/sec
      get-texts-at-range
        95.68 → 97.23 iterations/sec
      update-descendant
        72.57 → 77.09 iterations/sec
    rendering
      normal
        1.23 → 1.25 iterations/sec
    serializers
      raw-deserialize
        5.64 → 5.63 iterations/sec
      raw-serialize
        62.00 → 59.60 iterations/sec
    transforms
      delete-backward
        246.63 → 285.42 iterations/sec
      insert-text
        285.92 → 289.88 iterations/sec
      normalize
        87.54 → 105.78 iterations/sec (21% faster)
      split-block
        69.83 → 83.92 iterations/sec (20% faster)

Also this appears to change the nature of at least one bug #938, instead of deleting the previous line, it does not allow you to add spaces on a wrapped line. I was working on a solution to this separately but still had some things to iron out.

[kgdev]

Why we can remove isNative? Does draftjs do so too?

[ianstormtaylor]

Hey @conorcussell, nice!

I added one comment, but I think more of the onBeforeInput logic can be removed, which will further improve performance since that trigger fires a ton.

For testing, I think the best way to do it would be to use the Chrome Dev Tools performance stuff, and the "DEV: Large" (or similarly named) example that has tons of text, and just do a lot of typing individual characters while the performance metrics are collecting and then analyze. (You might even be able to feel the difference without analyzing to know immediately whether it slows things down a lot having removed it.)

Depending on what the outcome is, there might be a few new render code paths that need to be tweaked with the isNative logic gone now. If you get it working towards that though I can clone this branch and do some tests of my own. Thanks for taking up this change!

[ianstormtaylor]

I think nextChars, nextText (and probably others) are now unused so we don't need to compute them too? There might be a handful more lines of code we can remove here which will improve performance when checking further.

[ianstormtaylor]

@kgdev I'm not sure we can remove it, but it's a hunch I wanted to investigate. One thing we do which Draft doesn't is that we memoize the Immutable.js objects, so many of their methods are very low cost to call for the second time if nothing has changed. I have no idea if it's something we can avoid doing, but it was worth an investigation since isNative adds lots of complexity.

[conorcussell]

Thanks @ianstormtaylor I removed the rest of the code we don't need from that method. Will try and take a look at the perf impact in devtools and reply with my findings when I get a chance.

[ianstormtaylor]

Hey @conorcussell thanks again for kicking this off! I took your changes and added some more on #1088, so I'm going to close this now. Cheers!