regl icon indicating copy to clipboard operation
regl copied to clipboard

Plotly - device-independent and stable codegen + codegen cache.

Open yujin-wu opened this issue 3 years ago • 4 comments

Issue

https://github.com/regl-project/regl/issues/638

  • It is currently not possible to have regl run in a CSP strict environment due to violation of unsafe-eval. This change allows developers to pre-generate shaders for their apps and make them CSP compliant.
  • In applications using regl, especially ones using many shaders, the same blocks of code can be generated multiple times but end time a new almost-duplicate (or fully duplicate) function is generated.

Changes

  • Made generated code stable - the same code is generated for the same shader regardless of device, construction order, gl attributes or limits. -- added "stable link" capability to link runtime-dependent values. -- rolled-up the vertex array loop.
  • global cache of generated code, indexed by sha256 of code.

This makes it possible for developers to create CSP compliant apps using regl, roughly by:

  1. generate stable code of all shaders used by the app, by running the app and invoking all the shaders.
  2. Extract the generated code from the __regl_codegen_cache, and save into application source.
  3. On application initialization, load the generated code into the __regl_codegen_cache. Regl will avoid codegen for those functions.

Impact on performance

  • There is small performance hit on codegen time, mainly due to a sha256 pass.
  • There is minimal impact (or even improvement) on execution performance for a single shader - stable variable lookups take marginally longer, but the rolled-up vertex array loop runs faster in many cases.
  • There is performance improvement for applications using multiple regl shaders at a time - a lot of codegen now produce identical code, and those identical code return reused functions.

yujin-wu avatar Jan 19 '22 15:01 yujin-wu

Thanks for the PR! It would be great to fix the CSP issues! I don't currently have time for an in-depth review, but a couple things which jump out at me are:

  • Since regl doesn't require a browser, I don't think window.__regl_codegen_cache will work since window may not exist. I think any caching would need to be scoped to the specific regl instance.
  • It looks like crypto isn't currently handled correctly by rollup. I see a warning like this:
Treating 'crypto' as external dependency
No name was provided for external module 'crypto' in options.globals – guessing 'crypto'

with the result that dist/regl.js tries to call a function require to locate it, which doesn't exist:

(function (global, factory) {
    typeof exports === 'object' && typeof module !== 'undefined' ? module.exports = factory(require('crypto')) :
    typeof define === 'function' && define.amd ? define(['crypto'], factory) :
    (global.createREGL = factory(global.crypto));
}(this, (function (crypto) { 'use strict';

A way this has been handled in the past was to write a browserify transform which could be run to transform a dynamic call into a static call. See, for example cwise-transform.js which replaces the entry point to replace dynamic calls with code generation to static calls with the generated code. Unfortunately, the ecosystem is fragmented and browserify transforms can cause lots of headaches with various bundlers.

If anything though, I think it'd be maybe preferable to replace interaction with a global window.__regl_codegen_cache with proper methods like regl.getCachedCode() and regl.preloadCachedCode(), after which you could load it however you like.

rreusser avatar Jan 24 '22 23:01 rreusser

Ah, I think static-module was the module which replaces module usage with inline expressions. Plotly includes it via brfs, though I'm not sure if it's actively used already. https://github.com/plotly/plotly.js/blob/master/package-lock.json#L1156

rreusser avatar Jan 24 '22 23:01 rreusser

Thanks for taking a look!

I have added the cache to window before because I thought maybe applications have libraries that instantiate regl itself which wasn't accessible to the application. But alas I haven't seen an example of that, and it is nicer to keep it contained so as you've said I've changed that now.

I didn't even realize the crypto issue until you mentioned it, because the resulting code did happen to work wherever web cryptography existed :P. But yeah it ought to be fixed - I am still replacing it with a dependency-free safe hash function (hopefully lighter than sha256). I am still considering whether using an attackable hash/digest function to index cached code is a security concern here, if so I may end up having to include a crypto library just for this purpose.

yujin-wu avatar Jan 26 '22 15:01 yujin-wu

So great that this is being worked on, scattergl working with a strong CSP will be awesome!

CallumNZ avatar Mar 06 '22 21:03 CallumNZ