Enable strict mode in js

[TheOneric]

In my crude and limited tests, I didn't see any relevant performance difference not within run-to-run variance (i.e. also no degradation), but I'm not familiar with benchmarking js.
@ThaUnknown: would you like to take a look and see, if this has a negative, positive or no performance impact?

[ThaUnknown]

In my crude and limited tests, I didn't see any relevant performance difference not within run-to-run variance (i.e. also no degradation), but I'm not familiar with benchmarking js. @ThaUnknown: would you like to take a look and see, if this has a negative, positive or no performance impact?

as I already mentioned in #138, most performance lost is in constructing data objects like ImageData and Uint8ClampedArray and converting the bitmaps to images, so strict mode doesn't offer much performance benefits here anyways

you can insanely easily minimize the JS overhead to close to 0 by patching the resizable buffer function of EMSC so you don't need to construct the data array, and instead just reference it or if you don't want to implement offscreen rendering just slice the buffer exposed my EMSC and move bitmap conversion to WASM as you can see by:

[decode time and draw time is JS, rest is WASM]

[TheOneric]

With strict mode I'm mostly interested in catching otherwise silent bugs early. I would like to ensure there’s no performance penalty; a performance improvement would ofc be a neat side effect.

Since my tests had high run-to-run variance and I'm not familiar with benchmarking JS, but you appear to be, it would be reassuring if you could verify there’s no penalty. I don't want to coerce you into anything though, if you are not interested just say so and I'll see what else I can do.

[ThaUnknown]

With strict mode I'm mostly interested in catching otherwise silent bugs early. I would like to ensure there’s no performance penalty; a performance improvement would ofc be a neat side effect.

Since my tests had high run-to-run variance and I'm not familiar with benchmarking JS, but you appear to be, it would be reassuring if you could verify there’s no penalty. I don't want to coerce you into anything though, if you are not interested just say so and I'll see what else I can do.

strict mode only really changes things if there's a JS function called say 100k times in a few seconds, then it might be marginally slower
when reading the chromium source I recall seeing basic if checks, so treat any enforcement by strict mode an extra if check
supposedly it can offer benefits to V8, for it to optimise things... better, but that's NOT what it was made for [from my understanding]

you shouldn't really use strict mode to catch bugs, it won't do that for you, not even in the slightest, from the sounds of it you'll be interested in a linter such as eslint preferably with some strict configuration. I personally use standard.js, as it's as you put it "good at catching bugs early"
You could use standard, then before compiling JS or even on any commit/PR have a lint test using an npm script which would do something along the likes of standard src/worker.js which will fail the CI which ran it
see: https://binary-studio.com/2021/12/21/lint-your-project-with-github-actions/ [first good result I found on google]

[TheOneric]

you shouldn't really use strict mode to catch bugs, it won't do that for you, not even in the slightest,

It will in some cases, as I can personally account for :) (for example typos in variable assignments not silently creating new global variables)
Also, Modules use strict mode by default and some use JSO inside Modules, as evident by past fixes for strict mode having been sent to JSO. If we enable strict mode in our own default non-module setup *(assuming it has no perf penalty), we’ll avoid again introducing something which doesn't work with strict mode in the future.

but that's NOT what it was made for [from my understanding]

Exactly, that’s why this isn't the focus of this change.Yet performance degradations (due to additional checks) are undesireable so this must be tested beforehand (i.e. does the better optimisation due to not having to account for some features and side effects outweigh or neutralise the additional checks; or are the additional checks insignificant?)

The essence is, it would be really great if you could leverage your js benchmark knowledge and check if this casuses perf degradation (bad), perf improvement or no change (both good).

[ThaUnknown]

If you provide me with the built files I could benchmark this, FYI I'm on IRC right now, we can chat there

[TheOneric]

If you provide me with the built files I could benchmark this

Sure, here’s a version of current master (after the build cleanup patches were merged) and one with this patch applied: strict-nonstrict-comp.zip.
The only difference apart from a changed package_uuid are three "use strict";s (one per worker version and one in subtitles-octopus.js).

---

OffTopic:

FYI I'm on IRC right now,

I am now, but you aren't anymore ^^`. Fyi, if you plan to use IRC more: there are IRC Bouncers which will let you stay connected and not miss messages even when your client(s) are offline, or alternatively using e.g. a XMPP<->IRC or Matrix<->IRC bridge will also serve as a sort of bouncer.
From quickly skimming through the backlog, it appears like af1dc9a might perhaps be of interest to you.

[ThaUnknown]

strict mode was ~1% slower which could be chalked up to measuring error from inaccurate/missing samples

[TheOneric]

Thanks! It seems this is good to merge then.