Hi, Did anyone had the opportunity to look into this bug report and in the proposed fix?
Sincerely, Saulo On Fri, Aug 12, 2016 at 10:55 PM, Saulo Araujo <[email protected]> wrote: > Hi, > > I believe there is a substantial memory leak in the JavaScript runtime of > Ur/Web. To see it happening: > > 1) visit http://timesheet-ur.sauloaraujo.com:8080/TimeSheet/application > with Google Chrome > 2) open the developer tools by pressing f12 > 3) click in the "Profiles" tab > 4) click in the "Take Heap Snapshot" radio button > 5) click in the "Take Snapshot" button > 6) type Detached in the "Class filter" text box (you will see that the > heap has about 7MB and Detached DOM trees are retaining 0% of the memory - > https://snag.gy/2nF9fz.jpg) > > 7) click 10 times in the foward icon (the one on the right of the "Date" > header in the time sheet application) > 9) click in the "Take Snapshot" button > 10) type Detached in the "Class filter" text box (you will see that the > heap has about 43MB and Detached DOM trees are retaining 16% of the memory > - https://snag.gy/Crc3Vg.jpg) > > 11) click 10 times in the foward icon (the one on the right of the "Date" > header in the time sheet application) > 12) click in the "Take Snapshot" button > 13) type Detached in the "Class filter" text box (you will see that the > heap has about 78MB and Detached DOM trees are retaining 17% of the memory > - https://snag.gy/P5BLhq.jpg) > > I have spent quite some time investigating this memory leak and I believe > the problem is in the function recreate inside the function dyn: > > function dyn(pnode, s) { > if (suspendScripts) > return; > > var x = document.createElement("script"); > x.dead = false; > x.signal = s; > x.sources = null; > x.closures = null; > > var firstChild = null; > > x.recreate = function(v) { > for (var ls = x.closures; ls; ls = ls.next) > freeClosure(ls.data); > > var next; > for (var child = firstChild; child && child != x; child = next) { > next = child.nextSibling; > > killScript(child); > if (child.getElementsByTagName) { > var arr = child.getElementsByTagName("script"); > for (var i = 0; i < arr.length; ++i) > killScript(arr[i]); > } > ... > > Note that recreate only kills <script> nodes . Therefore, <input>, > <textarea> and <select> nodes created through <ctextbox>, <ctextarea> and > <cselect> will continue to be in the dyns lists of its sources. To fix this > memory leak, I propose changing the function dyn to > > function dyn(pnode, s) { > if (suspendScripts) > return; > > var x = document.createElement("script"); > x.dead = false; > x.signal = s; > x.sources = null; > x.closures = null; > > var firstChild = null; > > x.recreate = function(v) { > for (var ls = x.closures; ls; ls = ls.next) > freeClosure(ls.data); > > var next; > for (var child = firstChild; child && child != x; child = next) { > next = child.nextSibling; > > killDyns(child) > ... > > Below is the function killDyns. > > function killDyns(node) { > var nodeIterator = document.createNodeIterator(node, > NodeFilter.SHOW_ELEMENT, function (node) { > return node.dead !== undefined && node.dead === false > }) > var node = nodeIterator.nextNode(); > while (node) { > killScript(node); > node = nodeIterator.nextNode(); > } > } > > killDyns traverses all descendants of a node that have a dead attribute > equal to false and kills them with killScript. Therefore, killDyns may be > less performant than the code it substitutes, but I believe killDyns is > less prone to memory leaks than the original code. There is another small > change to be done in urweb.js. You can see all changes in > https://github.com/saulo2/urweb/commit/dcd280c85595ceee60a4fb78a3bfaf > 413a9eb7b8#diff-867e8dfbbc36419eefc0dfdf9db32883 > > I did not make a pull request yet because I would like to know the opinion > of the Ur/Web comitters about this fix. Maybe there is something that can > be improved. Maybe there is a new bug in it :) In any case, repeating the > previous steps with the proposed changes, I get the following results: > > 6) type Detached in the "Class filter" text box (you will see that the > heap has about 7MB and Detached DOM trees are retaining 0% of the memory - > https://snag.gy/NicJow.jpg) > 10) type Detached in the "Class filter" text box (you will see that the > heap has about 15MB and Detached DOM trees are retaining 0% of the memory - > https://snag.gy/nuVfhg.jpg) > 13) type Detached in the "Class filter" text box (you will see that the > heap has about 21MB and Detached DOM trees are retaining 0% of the memory - > https://snag.gy/aPeUuK.jpg) > > The new results suggest that there is another memory leak in the > JavaScript Runtime. However, I have not looked into it yet. > > Sincerely, > Saulo >
_______________________________________________ Ur mailing list [email protected] http://www.impredicative.com/cgi-bin/mailman/listinfo/ur
