[Review] Optimal performance & supportability

[willstocks]

I'm going to keep this issue open for a while, just until the repo has settled a bit. Any performance-related improvements, pop a comment down below and I will add them to this list.

Current areas for improvement:

• Multiple for loops (resolved in [RELEASE] v0.0.5 #11)
• Multiple ifs (some nested unnecessarily) (resolved in [RELEASE] v0.0.5 #11)
• console.log alternative? Stripped most out of them from the primary scripts (non-min and min) - only concern here is impact to mobile devices. No "data" is being shared at all! (resolved in [RELEASE] v0.0.5 #11)
• A fair bit of repeated code in the loadMyScript function - move into helper functions? (resolved in [RELEASE] v0.0.5 #11)
• Repeated code in init function (resolved in [RELEASE] v0.0.5 #11)
• Always having to loop back to "core" function - can we just go step by step? (resolved in [RELEASE] v0.0.5 #11)
• Large number of "filler" variables (resolved in [RELEASE] v0.0.5 #11)
• Appending script to body - is the current method the best method for all devices (not just desktop, but mobile too)? (https://jsperf.com/create-element-direct-append-vs-get-elements/1)
• Variables: var, let or const?
• if's - are they being handled in the best way in general? switch/case or continue with if else (https://jsperf.com/switch-case-vs-if-else/13)
  • Windows 10, Chrome 71: if else = slowest vs object = fastst (switch-case close second)
• There's a set of nested ifs within the checking function. See https://github.com/willstocks-tech/dynamically-polyfill-features-for-a-script/blob/develop/dynamicpolyfill.js#L40+L46
• Complete some benchmarking on each function (and the script overall) just to see if there are any leaks/bottlenecks (see [Review] Optimal performance & supportability #12 (comment))
• dynamicPolyfill has become potentially "Cognitively Complex" (see: https://codeclimate.com/github/willstocks-tech/dynamically-polyfill-features-for-a-script/pull/19)

[willstocks]

Multiple for loops

MOSTLY resolved as of #11. Where possible, for loops iterating through arrays have been converted to Array.forEach.

---

console.log alternative? Stripped most out of them from the primary scripts (non-min and min) - only concern here is impact to mobile devices. No "data" is being shared at all!

Resolved as of #11 - stripped most out of the primary scripts and deprecated the consolemessages version. Only console messages that remain relate to errors.

---

A fair bit of repeated code in the loadMyScript function - move into helper functions?

Resolved as of #11 - all repeated code has been refactored either into helper functions or removed.

---

Repeated code in init function

Resolved as of #11 - all repeated code has been refactored into micro-helper function or removed.

---

Always having to loop back to "core" function - can we just go step by step?

Partial resolution as of #11 - some functions now go from one, to the next, rather than core -> function -> core -> function -> core -> function

---

Large number of "filler" variables

Resolved as of #11 - all unnecessary vars removed, although there may be opportunity within the checkPolyfills function?

---

Appending script to body - is the current method the best method for all devices (not just desktop, but mobile too)?

---

Variables: var, let or const?

We're using const where the values are constant and don't change, and var for where they could change. Open to further suggestion (although I don't think this is going to affect performance in any way at all (if it does, we're talking nano-to-low-microseconds). Happy to be proven wrong!

---

if's - are they being handled in the best way in general? switch/case or continue with if else

---

There's a set of nested ifs within the checking function.

---

Complete some benchmarking on each function (and the script overall) just to see if there are any leaks/bottlenecks

See this comment, that details some basic, high-level benchmark stats for a couple of common browsers (that I have access to).

---

dynamicPolyfill has become potentially "Cognitively Complex"

[willstocks]

End to end stats (using console.time(), console.timeEnd() at the start and end of the script respectively):

• Native features: ~180ms first run, ~40ms consecutive
• Non-native features (2x): ~1.6s first run, 200ms consecutive

It's calling out for:

• polyfill
• third party script

That results in the 1.6s first run - as soon as they're in the browser cache, things are better!

All stats gathered via Codepen: https://codepen.io/willstocks_tech/pen/YBEzLW

[willstocks]

Using the following as a benchmark (using performance.mark and performance.measure), consecutive runs: https://codepen.io/willstocks_tech/pen/xBwJpW?editors=0012#0

Windows 10, Chrome 72 (all native features)

1st run: 180ms (empty cache)
2nd run: 137ms (cache cleared (Chrome Empty Cache and Reload))
3rd run: 19ms
4th run: 13ms
5th run: 9ms
6th run: 20ms

Windows 10, IE11

1st run: 316ms (empty cache)
2nd run: 1153ms (cache cleared)
3rd run: 117ms
4th run: 98ms
5th run: 134ms
6th run: 158ms

Windows 10, Edge 40.15063.674.0 (EdgeHTML 15.15063)

1st run: 358ms (empty cache)
2nd run: 585ms (cache cleared)
3rd run: 120ms
4th run: 89ms
5th run: 74ms
6th run: 71ms