espruino / BangleApps

Bangle.js App Loader (and Apps)
https://banglejs.com/apps
MIT License
496 stars 1.17k forks source link

[ContourClock] suboptimal rendering - font characters in json files are not pretokenized #3649

Open fanoush opened 2 weeks ago

fanoush commented 2 weeks ago

Affected hardware version

Bangle 2

Your firmware version

2v24

The bug

All the digits are stored as base64 strings, look e.g. at https://github.com/espruino/BangleApps/blob/master/apps/contourclock/fonts/font-Anton.json and then in drawClock() called each minute (?) the file is parsed and decoded here https://github.com/espruino/BangleApps/blob/master/apps/contourclock/lib.js#L19 over and over again.

Either the selected font file could be parsed once in the code when font selection is made and stored in new file (contourclock-font,json ?) with the array somehow stored as binary or maybe the font json files could contain atob() inside so that it gets pretokenized by BangleApps uploader automagically(?) This is mainly question to @gfwilliams - would such json with atob() inside be pretokenized at upload time and if not, would JSON.parse handle it at parse time as valid json? If not how to solve it - maybe eval on the json would work then?

Installed apps

No response

gfwilliams commented 2 weeks ago

Wow, that sucks. Surely this should just happen when the clock starts up? That would be an easy fix.

The JSON files won't be pretokenised or anything, so the simplest 'fix' would be to make them JS modules instead, and to not do {"width" : "53", "buffer":"base64... but {"width" : "53", "buffer":atob("base64") so Espruino could store the pre-decoded data as a raw string.

xxDUxx commented 1 week ago

I just tried a new approach. Font files are parsed when you select a font, then the images are stored in the settings file. Rendering time is now 250ms instead of 50ms, but RAM usage is now 4680 all the time, because the settings object stays in RAM. I wonder which way i should optimize this - RAM usage, storage or performance.

gfwilliams commented 1 week ago

... you could try what I suggested with JS modules and get low ram usage and pretty good performance?

Storing the contents in settings JSON feels like it's not really too different from what was there before. It's still a JSON object that's parsed - it's just whether you decide to parse it once when the app loads, or every time you draw something.

fanoush commented 1 week ago

you could try what I suggested with JS modules

So it is just about adding atob inside the json file, renaming it from .json to .js and loading it here via var font = require(fontFile); instead? No 'export' needed inside if it is just one object?

but still a bit better would be to load the font variable once somewhere in app.js instead of the drawClock

gfwilliams commented 1 week ago

So it is just about adding atob inside the json file, renaming it from .json to .js and loading it

If you're using eval(require("Storage").read(..))

If you want to use module then you need export, but using eval is probably nicer actually

fanoush commented 1 week ago

Anyway, the minimalistic improvement is to cache the font by fontIndex passed to drawclock here https://github.com/espruino/BangleApps/blob/master/apps/contourclock/lib.js#L11 something like this:

var is12;
var font;
var index;
function getHours(d) {
  var h = d.getHours();
  if (is12===undefined) is12 = (require('Storage').readJSON('setting.json',1)||{})["12hour"];
  if (!is12) return h;
  return (h%12==0) ? 12 : h%12;
}

exports.drawClock = function(fontIndex) {
  var digits = [];
  if (fontIndex!=index) {
    var fontFile=require("Storage").read("contourclock-"+Math.abs(parseInt(fontIndex+0.5))+".json");
    if (fontFile==undefined) return(false); //exit if font file not found
    font = JSON.parse(fontFile);
    index=fontIndex;
  }

This way it is parsed only once as long as the index passed to drawClock is same. So it is only slow at app startup but does not consume cpu by reloading font at every draw.

but using eval is probably nicer actually

But then we are back to first question, when will apploader optimize the file? it is only about filename change json->js? or is it only done on files in metadata here https://github.com/espruino/BangleApps/blob/master/apps/contourclock/metadata.json#L13 ? Fonts are uploaded here https://github.com/espruino/BangleApps/blob/master/apps/contourclock/custom.html#L59

BTW maybe all fonts could be uploaded and custom html could be removed, maybe it is not so much data after all ? EDIT: but this belongs to issue #3648

xxDUxx commented 1 week ago

Parsing the font file takes about 200ms, which is acceptable on startup in my opinion.

BTW maybe all fonts could be uploaded and custom html could be removed, maybe it is not so much data after all ? EDIT: but this belongs to issue #3648

One font takes up about 20kB, uploading all fonts uses a significant portion of the available flash. It would also slow down updating unnecessarily. The custom.html needs a major rework anyway, so expect an update in the next few weeks...

fanoush commented 1 week ago

One font takes up about 20kB, uploading all fonts uses a significant portion of the available flash. It would also slow down updating unnecessarily.

yes, maybe it is too much, but if you would store it with atob then it would be 2/3 of that and with heatshrink applied it would be even (much?) smaller. and maybe decompressing heatshrink would not be slower than current atob

fanoush commented 1 week ago

Anyway, the minimalistic improvement is to cache the font

oh, sorry, when looking at the code, better would be to cache the digits array instead, which comes in next lines here https://github.com/espruino/BangleApps/blob/master/apps/contourclock/lib.js#L14

this works for me

diff --git a/apps/contourclock/lib.js b/apps/contourclock/lib.js
index c4f927953..161c313b1 100644
--- a/apps/contourclock/lib.js
+++ b/apps/contourclock/lib.js
@@ -1,4 +1,24 @@
 var is12;
+var digits;
+var index;
+var fontName;
+function cacheDigits(fontIndex){
+  let fontFile=require("Storage").read("contourclock-"+Math.abs(parseInt(fontIndex+0.5))+".json");
+  if (fontFile==undefined) return; //exit if font file not found
+  let font = JSON.parse(fontFile);
+  digits=[];
+  for (var n in font.characters) {
+  digits.push({width: parseInt(font.characters[n].width),
+    height: font.size,
+    bpp: 2,
+    transparent: 1,
+    buffer:E.toArrayBuffer(atob(font.characters[n].buffer))});
+  }
+  if (n!=10) return; //font file seems to be invalid
+  fontName=font.name;
+  index=fontIndex;
+}
+
 function getHours(d) {
   var h = d.getHours();
   if (is12===undefined) is12 = (require('Storage').readJSON('setting.json',1)||{})["12hour"];
@@ -7,18 +27,10 @@ function getHours(d) {
 }

 exports.drawClock = function(fontIndex) {
-  var digits = [];
-  fontFile=require("Storage").read("contourclock-"+Math.abs(parseInt(fontIndex+0.5))+".json");
-  if (fontFile==undefined) return(false); //exit if font file not found
-  var font = JSON.parse(fontFile);
-  for (var n in font.characters) {
-    digits.push({width: parseInt(font.characters[n].width),
-      height: font.size,
-      bpp: 2,
-      transparent: 1,
-      buffer:E.toArrayBuffer(atob(font.characters[n].buffer))});
-    }
-    if (n!=10) return (false); //font file seems to be invalid
+  if (fontIndex!=index) {
+    cacheDigits(fontIndex);
+    if (fontIndex!=index) return false;
+  }
     var x=0;
     var y = g.getHeight()/2-digits[0].height/2;
     var date = new Date();
@@ -53,5 +65,5 @@ exports.drawClock = function(fontIndex) {
       g.setColor(fg);
       g.setBgColor(bg);
     }
-    return font.name;
+    return fontName;
   }

EDIT: and also BTW all those d1-d5,w1-w5 variables could be defined with let/var as local variables inside drawClock, now they are global variables (just connect to watch and type its name to see)

fanoush commented 1 week ago

yes, maybe it is too much, but if you would store it with atob then it would be 2/3 of that and with heatshrink applied it would be even (much?) smaller. and maybe decompressing heatshrink would not be slower than current atob

just tested it with making font into module and also tested heatshrink on thedata

require("Storage").list().forEach(function(v){print(v,require("Storage").read(v).length)})
font-Anton.json 18095
anton.json 18004
anton.js 13585

anton.json is just uploaded font-Anton.json from ide as is with tokenization anton.js is font changed into js file with exports and atob like

exports = {
  "name":"Anton",
  "size":"100",
  "characters":[
    {"width" : "53", "buffer":atob("VVVVVV
....

it can be seen that the pretokenization saves space by tokenizing atob into binary strings. Also here are shrinked values of the digits data itself

var f=require("anton.js")
f.characters.forEach((c)=>{c.sbuffer=require("heatshrink").compress(c.buffer)})
f.characters.forEach((c)=>print(c.buffer.length,c.sbuffer.length))
1325 296
850 130
1300 249
1325 435
1375 387
1325 380
1325 450
1275 131
1300 341
1325 436
500 27

so even in current state you could compress the arrays and then when loading digits decompress it back. that would save quite some storage space and with recent changes would be done only once.

fanoush commented 1 week ago

I have implemented the module way and converted only one font to this mode (=loaded "contourclock-3.json" into IDE from storage, added exports= and atob() around base64 strings and uploaded as contourclock-3.js) and it falls back on old way with all others

function cacheDigits(fontIndex){
  var font;
  let fn = "contourclock-"+Math.abs(parseInt(fontIndex+0.5));
  try {
    // new way
    font=require(fn+".js");
    digits=[];
    for (var n in font.characters) {
      digits.push({width: parseInt(font.characters[n].width),
        height: font.size,
        bpp: 2,
        transparent: 1,
        buffer:font.characters[n].buffer});
      }
    } catch {
    // fallback to old way
    let fontFile=require("Storage").read(fn+".json");
    if (fontFile==undefined) return; //exit if font file not found
    font = JSON.parse(fontFile);
    digits=[];
    for (var n in font.characters) {
    digits.push({width: parseInt(font.characters[n].width),
      height: font.size,
      bpp: 2,
      transparent: 1,
      buffer:E.toArrayBuffer(atob(font.characters[n].buffer))});
    }
  }
  if (n!=10) return; //font file seems to be invalid
  fontName=font.name;
  index=fontIndex;
}

and when you go to preferences and slide between fonts is is very noticeable that the one converted is much faster to show

gfwilliams commented 1 week ago

Have you seen https://www.espruino.com/Bangle.js+Performance#measuring-performance - you might be able to get some numbers

fanoush commented 1 week ago

Thanks for those tips. I have just added var _t = Date.now(); to beginning of drawClock and then print("cacheDigits",fontName,Date.now()-_t); into the if that is possibly caching and then print("drawClock()",Date.now()-_t); to the end of drawClock() method.

I have also made font 1 = "Oswald" into module and compressed the array via heatshrink. font 3 = "Beba Neue" is only module with no compression. Here is output of sliding in preferences few times to right, left, right. then keeping "Bebas Neue" and letting the clock run for few minutes

cacheDigits Bebas Neue 40.80200195312
drawClock() 99.06005859375
cacheDigits Dekko 225.28076171875
drawClock() 273.89526367187
cacheDigits DIN Alternate 192.41333007812
drawClock() 237.51831054687
cacheDigits Impact 234.68017578125
drawClock() 293.5791015625
cacheDigits DIN Alternate 182.70874023437
drawClock() 225.341796875
cacheDigits Dekko 220.85571289062
drawClock() 269.287109375
cacheDigits Bebas Neue 44.28100585937
drawClock() 108.67309570312
cacheDigits Anton 222.56469726562
drawClock() 277.7099609375
cacheDigits Oswald 155.73120117187
drawClock() 207.763671875
cacheDigits Teko 212.158203125
drawClock() 263.85498046875
cacheDigits Teko 205.35278320312
drawClock() 254.45556640625
cacheDigits Oswald 159.11865234375
drawClock() 208.3740234375
cacheDigits Teko 206.298828125
drawClock() 255.18798828125
cacheDigits Teko 209.38110351562
drawClock() 261.16943359375
cacheDigits Oswald 166.44287109375
drawClock() 218.505859375
cacheDigits Anton 214.63012695312
drawClock() 267.02880859375
cacheDigits Bebas Neue 42.236328125
drawClock() 102.99682617187
cacheDigits Dekko 249.3896484375
drawClock() 293.18237304687
cacheDigits DIN Alternate 181.5185546875
drawClock() 223.35815429687
cacheDigits Dekko 215.91186523437
drawClock() 262.29858398437
cacheDigits Bebas Neue 41.53442382812
drawClock() 92.55981445312
cacheDigits Bebas Neue 43.3349609375
drawClock() 96.98486328125
drawClock() 47.94311523437
drawClock() 50.26245117187
drawClock() 49.86572265625
drawClock() 52.3681640625

So it depends on font images a bit but cacheDigits is like 40 vs 155 vs 200ms for pretokenized vs pretokenized+shrinked vs plain text json, then drawing time is another 50ms. Original before caching was like 250-290ms each minute.

Font sizes are

require("Storage").list().forEach(function(v){if (v.startsWith("contourclock-"))print(v,require("Storage").read(v).length)})
contourclock-0.json 17253
contourclock-1.json 18492
contourclock-2.json 18095
contourclock-3.json 16214
contourclock-4.json 19170
contourclock-5.json 14715
contourclock-6.json 19207
contourclock-3.js 12177
contourclock-1.js 3739

my current cacheDigits() loading all 3 types is

function cacheDigits(fontIndex){
  var font;
  let fn = "contourclock-"+Math.abs(parseInt(fontIndex+0.5));
  var n;
  try {
    font=require(fn+".js");
    digits=[];
    n=font.characters.length-1;
    let getBuff=function(c){if (c.shrink) return require("heatshrink").decompress(c.shrink); else return c.buffer;}
    font.characters.forEach((c)=> {
      digits.push({width: parseInt(c.width),
        height: font.size,
        bpp: 2,
        transparent: 1,
        buffer:getBuff(c)});
      })
    } catch {
    let fontFile=require("Storage").read(fn+".json");
    if (fontFile==undefined) return; //exit if font file not found
    font = JSON.parse(fontFile);
    digits=[];
    for (n in font.characters) {
    digits.push({width: parseInt(font.characters[n].width),
      height: font.size,
      bpp: 2,
      transparent: 1,
      buffer:E.toArrayBuffer(atob(font.characters[n].buffer))});
    }
  }
  if (n!=10) return; //font file seems to be invalid
  fontName=font.name;
  index=fontIndex;
}

I have compressed font 1 like this

var f=JSON.parse(require("Storage").read("contourclock-1.json"))
f.characters.forEach((c)=>{print('"shrink":atob("'+btoa(require("heatshrink").compress(atob(c.buffer)))+'")')})

then loaded json into webide and replaced "buffer":"..." with "shrink":atob("..")

fanoush commented 1 week ago

If you want to use module then you need export, but using eval is probably nicer actually

BTW, I was using module because I did not know how to do it with eval. The IDE complains at upload if it the file is just object in {} with methods in JSON. However now after everything done I had an idea to wrap it into () and that works.

>{"x":5,"y":btoa("xx")}
Uncaught SyntaxError: Got ':' expected EOF
 at line 1 col 5
{"x":5,"y":btoa("xx")}
    ^
>({"x":5,"y":btoa("xx")})
={ x: 5,
  y: "eHg="
 }
>

So which is better / what is nicer about eval ? extern = + require() or wrapping file data into () and eval. Modules are cached in global['\xff'].modules is that good or bad? When possibly swiping between font in preferences it can grow but it is backed by flash so won't waste much memory(?)

gfwilliams commented 1 week ago

I had an idea to wrap it into () and that works.

Ahh - I forgot about that. eval is preferable because of the module cacheing as you noticed, but if you have to add "(" at runtime, it undoes some of the benefits because then the whole string has to be loaded into RAM so it can be appended to!

But I just had a thought! Actually Storage.readJSON is smart enough to decode flash strings:

>trace(require("Storage").readJSON("test.json"))
#198[r0,l1] Object {
  #202[r1,l2] Name String [1 blocks] "foo"    #200[r1,l0] FlashString [1 blocks] "Hello world this is a big str"
}

And you could use Espruino to pretokenise the JSON and write it back to the file - it's a bit of a hack though:

require("Storage").write("contourclock-0.json", eval('()=>{"ram"return '+require("Storage").read("contourclock-0.json")+"}")["\xFFcod"])

And it doesn't help with decoding the base64 either.

So I'd say maybe just rename "contourclock-0.json" to "contourclock-0.js" and wrap the JS in () with the atob in it too before it's uploaded from the app loader - then you can use eval on it and it'll be fast.

Personally I feel like 20k for a JSON file isn't much - I'd just store the binary data the "contourclock-0.js" and not bother trying to decompress it on the fly (because if you do that the data ends up in RAM again)

fanoush commented 1 week ago

Ahh - I forgot about that. eval is preferable because of the module cacheing as you noticed, but if you have to add "(" at runtime

you don't need to but it needs to be inside the storage file, then also the IDE does not complain at upload and something like var f=eval(require("Storage").read("anton2.js")) works when the file has ({..}) inside

EDIT: I mean you don't need to do var f=eval("("+require("Storage").read("anton2.js")+")")

fanoush commented 1 week ago

not bother trying to decompress it on the fly (because if you do that the data ends up in RAM again)

OTOH when you just have the clock running most of the day drawing the data every minute all the day, it may not be bad for the five g.drawImage(digits[xx],x,y); calls to actually read the data from RAM instead of going into SPI flash for it. So as long as the RAM is there it can be used it this way? It probably doesn't matter much either way. Well unless the clock runs in background while other memory intensive stuff runs on top of it?

gfwilliams commented 1 week ago

may not be bad ... to actually read the data from RAM instead of going into SPI

true, yes. And we're on Bangle.js 2 only with this app so I guess we do have the RAM available, so that would be fine