Thanks for spotting this problem. I've pushed a different fix that
makes dynamic input elements create <script> tags, so that the original
dyn() code finds them to garbage-collect. Does this change also seem to
fix the problem?
(I'd like to avoid using createNodeIterator(), which only became widely
deployed about 5 years ago. I think Ur/Web client-side reactive code
today will still work on browsers from about the year 2000, and I'd like
to keep it that way unless there's a very compelling reason to the
contrary.)
On 08/12/2016 09:55 PM, Saulo Araujo 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/dcd280c85595ceee60a4fb78a3bfaf413a9eb7b8#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
_______________________________________________
Ur mailing list
[email protected]
http://www.impredicative.com/cgi-bin/mailman/listinfo/ur
