[WIP] Bugfix / Performance: Memory leak - always allocating new memor…

…y for font characters (mono#64)

Bugfix / Performance: Memory leak - always allocating new memory for font characters

.ci/esy-build-steps.yml

@@ -11,3 +11,4 @@ steps:
     continueOnError: true
   - script: esy install
   - script: esy build
+  - script: esy b dune runtest

package.json

@@ -34,15 +34,16 @@
     "@opam/lwt_ppx": "^1.1.0",
     "@opam/js_of_ocaml": "*",
     "@opam/js_of_ocaml-lwt": "*",
-    "@opam/dune": "^1.5.0",
     "flex": "^1.2.2",
-    "@opam/js_of_ocaml-compiler": "*"
+    "@opam/js_of_ocaml-compiler": "*",
+    "rejest": "^1.2.0"
   },
   "peerDependencies": {
     "ocaml": "~4.6.0"
   },
   "devDependencies": {
     "ocaml": "~4.6.0",
-    "@opam/merlin": "*"
+    "@opam/merlin": "*",
+    "@opam/dune": "^1.5.0"
   }
 }

src/Core/Revery_Core.re

@@ -10,20 +10,30 @@ module Event = Reactify.Event;
 
 module Performance = Performance;
 
+module Memoize = {
+    type t('a, 'b) = 'a => 'b;
+
+    let make = (f: t('a, 'b)): t('a, 'b) => {
+        let tbl: Hashtbl.t('a, 'b) = Hashtbl.create(16);
+
+        let ret = (arg: 'a) => {
+            let cv = Hashtbl.find_opt(tbl, arg);
+            switch(cv) {
+            | Some(x) => x
+            | None =>
+                let r = f(arg);
+                Hashtbl.add(tbl, arg, r); 
+                r;
+            }
+        };
+        ret;
+    };
+}
+
 module Lazy = {
-  type innerFunc('a) = unit => 'a;
-
-  let make = (f: innerFunc('a)): innerFunc('a) => {
-    let v: ref(option('a)) = ref(None);
-
-    let ret = () =>
-      switch (v^) {
-      | Some(x) => x
-      | None =>
-        let out = f();
-        v := Some(out);
-        out;
-      };
-    ret;
+  type t('a) = Memoize.t(unit, 'a);
+
+  let make = (f: t('a)): t('a) => {
+      Memoize.make(f);
   };
 };

src/UI/FontRenderer.re

@@ -1,17 +1,20 @@
 open Reglfw.Glfw;
 open Fontkit;
 
-let getGlyph = (font: Fontkit.fk_face, codepoint: int) =>
-  /* TODO: Cache */
-  Fontkit.renderGlyph(font, codepoint);
+open Revery_Core;
 
-let getTexture = (font: Fontkit.fk_face, codepoint: int) => {
+
+let _getGlyph = Memoize.make(((font: Fontkit.fk_face, codepoint: int)) =>
+  Fontkit.renderGlyph(font, codepoint));
+
+let getGlyph = (font: Fontkit.fk_face, codepoint: int) => _getGlyph((font, codepoint));
+
+
+let _getTexture = ((font: Fontkit.fk_face, codepoint: int)) => {
   let glyph = getGlyph(font, codepoint);
 
   let {image, _} = glyph;
 
-  /* TODO: */
-  /* - Cache textures */
   /* - Create texture atlas */
   glPixelStorei(GL_PACK_ALIGNMENT, 1);
   glPixelStorei(GL_UNPACK_ALIGNMENT, 1);
@@ -26,6 +29,9 @@ let getTexture = (font: Fontkit.fk_face, codepoint: int) => {
   texture;
 };
 
+let _memoizedGetTexture = Memoize.make(_getTexture);
+let getTexture = (font: Fontkit.fk_face, codepoint: int) => _memoizedGetTexture((font, codepoint));
+
 type dimensions = {
   width: int,
   height: int,

src/UI/TextNode.re

@@ -5,6 +5,7 @@ module LayoutTypes = Layout.LayoutTypes;
 
 open Fontkit;
 open Reglm;
+open Reglfw;
 open Revery_Core;
 
 open ViewNode;
@@ -55,7 +56,11 @@ class textNode (name: string, text: string) = {
 
         let {width, height, bearingX, bearingY, advance, _} = glyph;
 
-        let _ = FontRenderer.getTexture(font, s.codepoint);
+        Glfw.glPixelStorei(GL_PACK_ALIGNMENT, 1);
+        Glfw.glPixelStorei(GL_UNPACK_ALIGNMENT, 1);
+
+        let texture = FontRenderer.getTexture(font, s.codepoint);
+        Glfw.glBindTexture(GL_TEXTURE_2D, texture);
         /* TODO: Bind texture */
 
         let glyphTransform = Mat4.create();

test/Core/MemoizeTests.re

@@ -0,0 +1,39 @@
+open Rejest;
+
+open Revery_Core;
+
+test("Memoize", () => {
+  test("Simple memoization", () => {
+    let v = ref(0);
+    let testFunction =
+      Memoize.make(_a => {
+        v := v^ + 1;
+        v^;
+      });
+
+    let t0 = testFunction(0);
+    let t1 = testFunction(0);
+
+    expect(t0).toEqual(1);
+    expect(t1).toEqual(1);
+  });
+
+  test("Memoizes multiple arguments", () => {
+    let v = ref(0);
+    let testFunction =
+      Memoize.make(a => {
+        v := v^ + 1;
+        a ++ string_of_int(v^);
+      });
+
+    let t0 = testFunction("a");
+    let t1 = testFunction("b");
+    let t2 = testFunction("a");
+    let t3 = testFunction("b");
+
+    expect(t0).toEqual("a1");
+    expect(t1).toEqual("b2");
+    expect(t2).toEqual("a1");
+    expect(t3).toEqual("b2");
+  });
+});

test/Core/dune

@@ -0,0 +1,5 @@
+(library
+    (name Revery_Core_Test)
+    (library_flags (-linkall))
+    (modules (:standard))
+    (libraries Revery_Core rejest))

test/TestRunner.re

@@ -0,0 +1 @@
+Rejest.run();

test/dune

@@ -0,0 +1,10 @@
+(executable
+    (name TestRunner)
+    (libraries
+        rejest
+        Revery_Core_Test
+            ))
+
+(alias
+    (name runtest)
+    (action (run ./TestRunner.exe)))