lightning-js / renderer

Lightning 3 Renderer
Apache License 2.0
57 stars 23 forks source link

Fail gracefully createNativeCtxTexture in WebGlCoreCtxTexture load #406

Closed CosmaVergari closed 1 month ago

CosmaVergari commented 1 month ago
wouterlucas commented 1 month ago

Try / Catch statements are notoriously dangerous for their performance impact, as essentially you're copying the stack just in case something might happen.

What is the error exactly and isn't this handled in the catch already?

CosmaVergari commented 1 month ago

I just made a change to address your point. The idea is to manage the error raised by createNativeCtxTexture here

wouterlucas commented 1 month ago

Ah yes, makes sense.

It would be a little cheaper to change the return value of https://github.com/lightning-js/renderer/blob/dd9461f55ae7a2946141af934c84d88335c937bb/src/core/renderers/webgl/WebGlCoreCtxTexture.ts#L248 to be a null in case it fails and then check the value at https://github.com/lightning-js/renderer/blob/dd9461f55ae7a2946141af934c84d88335c937bb/src/core/renderers/webgl/WebGlCoreCtxTexture.ts#L88 over a try/catch block.

It won't matter that much in the sunny day scenario, Chrome 64 on x20 is a little faster for the null check but on older WebKits null check is about 60% faster. See: https://jsperf.app/siqize/2 In the negative case, generating an error chain is very very expensive: https://jsperf.app/siqize

CosmaVergari commented 1 month ago

Thank you for the suggestion, I converted the raised error to a null return. The performance gain seems significant indeed

wouterlucas commented 1 month ago

Thanks for your contribution!