Bug - Cannot Reproduce Chat related memory problem

[roippi]

Can you explain how cloning a string from a strongly reachable object actually reduces the memory usage? If I'm not mistaken, your original request object is still reachable, right?

Yes you are mistaken, no the original request object is not reachable. Read through what I posted closer and perhaps look at the code if that doesn't make sense.

 

[Catch-22]

Yes you are mistaken, no the original request object is not reachable. Read through what I posted closer and perhaps look at the code if that doesn't make sense.

You are a hawk, I deleted my post after I realized there were some mistakes in it. I'm still not quite sure but I'm mulling over the changes. Did you notice a reduction in memory footprint after making the changes?

 

[roippi]

Not noticeably, no. Turns out that the historyevents were going into a size 20 rollinglinkedlist, so while not ideal the memory leak was self-limiting.

Poking around with heap dumps possibly points me towards RequestEditorKit as a possible source - i.e. from rendering HTML. I didn't find any large leaks in any chat-specific areas though picking through heap dumps is a bit of an art form (and I'm not much of an artist) so that's not a guarantee of anything.

 

[Catch-22]

Yes you are mistaken, no the original request object is not reachable.

Okay, after thinking things through I'm still confused. I don't understand how the request object doesn't remain reachable until getEntries() returns, at which point the request object goes out of scope and becomes eligible for garbage collection.

 

[roippi]

Okay, after thinking things through I'm still confused. I don't understand how the request object doesn't remain reachable until getEntries() returns, at which point the request object goes out of scope and becomes eligible for garbage collection.

The issue was not in the scope of getEntries.

		ChatManager.processMessages( entry.getChatMessages() );

This creates a reference:

ChatManager.clanMessages.add( message );

to each list element of entry.getChatMessages, which itself returns a field of entry, thus a strong reference to entry is maintained. entry itself used to reference the request object; now it only references a cloned String.

Any clearer?

 

[Catch-22]

Any clearer?

Not really, but I appreciate you taking time to explain it.

entry itself used to reference the request object; now it only references a cloned String.

This is the part that doesn't make sense. entry doesn't reference the request object, it references the request.responseText object, which is just a String and contains no reference back to request. As soon as request goes out of scope (ie. at the end of getEntries) it becomes eligible for GC and only the request.responseText String remains reachable (via the HistoryEntry).

 

[roippi]

This is the part that doesn't make sense. entry doesn't reference the request object, it references the request.responseText object, which is just a String and contains no reference back to request. As soon as request goes out of scope (ie. at the end of getEntries) it becomes eligible for GC and only the request.responseText String remains reachable (via the HistoryEntry).

responseText is not just a String, it is a field of the request object. Referencing a field of an object prevents the object from being eligible for GC.

ETA: this is my understanding of GC, but I could be wrong. Especially with Strings, which get interned and confuse me regularly.

 

[xKiv]

responseText is not just a String, it is a field of the request object. Referencing a field of an object prevents the object from being eligible for GC.

ETA: this is my understanding of GC, but I could be wrong. Especially with Strings, which get interned and confuse me regularly.

This is just wrong. That's not how objects work in java.
String is a string is a string. The object doesn't know that it's stored in some other object's field. It doesn't care about any of the places where it is stored. If that other object becomes unreachable, it will be (eventually) GC'd, losing its reference to the string ... and then the string will be GC'd, unless it's reachable from other places.
Lexically, request.responseText is an attribute of the request object, but the object in it is not an attribute or a field.
The object in it *could* have a backreference to the containing object (if there was a closure, for an overcomplicated example), but Strings don't (and can't) do that.
And interning just means that string literals (what you write in the source code) are stored only once and reused, right?
(also, java doesn't call it "fields", afaik - maybe in the reflection API?).

Also, I think your use of "strong reference" and "weak reference" is mistaken. Strong references only prevent object from being GC'd if it's reachable from stack. You seem to think they prevent if from being GC'd always. (which could be something like a "superstrong" reference - reference stored in a variable that never goes out of scope, like static attribute).

ETA: or maybe you are thinking in terms of C++, where request.responseText would be like ..
class Request {
...
public: char[] responseText;
...
};

Then you couldn't GC responseText without GC'ing the entire request (and it's not that it wouldn't be safe - GCing responseText by itself would be invalid, because it's not a complete chunk on the heap).
But you alsowouldn't be GC'ing anything, because doing GC in C++ is a pain. In any case.

 

[roippi]

I think I was mistaken, yes. Referencing fields (yes, Java calls them fields) doesn't prevent the rest of the object from being GC'd unless the object is somehow reachable from those fields. FWIW this is where I learned GC and you can see how the figure gave me that impression.

Also, I think your use of "strong reference" and "weak reference" is mistaken. Strong references only prevent object from being GC'd if it's reachable from stack. You seem to think they prevent if from being GC'd always. (which could be something like a "superstrong" reference - reference stored in a variable that never goes out of scope, like static attribute).

No, that's not how I was using strong reference.

ETA: also, while I'm in ChatFrame, I notice that there appears to be some broken handling for KEYUP/KEYDOWN command history stuff. From the look of it it was never fully implemented, no? Guess I'll fix that while I'm in there.

Edit2: okay I've officially pinned down one leak, but it basically looks like it's Swing's fault. I'll see if there's a workaround, though. Not the first time we've had to do that with Swing.

Edit3: there's a leak in the HTML rendering pipeline somewhere, but it's certainly not on the order of what the OP is reporting. I figure about 5-10kb leaked over the 6 or so hours I had chat open. Certainly not enough to bring anyone's system to a standstill.

 

[Catch-22]

I did some visibility changes and a few language migrations, hopefully it's helpful to whoever is looking further into this.

Also, ETA to me means "estimated time of arrival", any hip kids wanna fill me in on the lingo?

 

[lostcalpolydude]

Also, ETA to me means "estimated time of arrival", any hip kids wanna fill me in on the lingo?

Edit(ed) to add.

 

[roippi]

So I'm basically at the point where I can't reproduce the bug. Looking for small memory leaks is very difficult, and I can't seem to get memory allocation to rise above 70 or 80k despite leaving chat open for much longer than the OP.

OP, if you are interested in doing a little work towards having this fixed, I can walk you through installing a JDK and taking a heap dump. Otherwise I can't help.

 

[Winterbay]

I do have the JDK installed and have noticed similar behaviour. In which way would you want me to generate a dump?

 

[roippi]

• Open mafia and open chat
• Open jdkInstallationPath/bin/jvisualvm.exe
• choose mafia
• Go to Sampler, click Memory
• click "perform GC", wait 3-4 seconds
• click "heap dump", save the .hprof file in the upper left

then let chat run for 5+ hours, do another GC, then take another heap dump.

That's gonna be around 150MB of files to send me, I imagine one of the you-upload-it services out there will be necessary. You might not want to publicly post links to them as technically there's info in there that you wouldn't want public. (though it's way more effort to dig it out than it's worth)

 

[Winterbay]

I'll try that when I get back home and send you a Dropbox-link when it's been uploaded.

 

[slyz]

I usually leave Mafia connected and the computer on during the night. Tonight I got a ~2MB debug log, with this being repeated every 8 seconds:

=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
         KoLmafia v15.9 r11957, Windows XP, Java 1.6.0_39
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Please note: do not post this log in the KoLmafia thread. If you
 would like the dev team to look at it, please write a bug report
 at kolmafia.us. Include specific information about what you were 
 doing when you made this and include the log as an attachment.
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
 Timestamp: Sat Mar 09 09:05:04 CET 2013
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=


Unexpected error, debug log printed.
class java.lang.NullPointerException: null
java.lang.NullPointerException
	at java.lang.String.<init>(Unknown Source)
	at net.sourceforge.kolmafia.chat.ChatPoller.getEntries(ChatPoller.java:179)
	at net.sourceforge.kolmafia.chat.ChatPoller.run(ChatPoller.java:74)

 

[Alhifar]

That particular error has been happening any time I have the chat interface open during rollover, or during a connection loss to the KoL servers

 

[slyz]

It cropped up for me with this revision, I never had it before.

It's probably fixed in r11959 though, thanks Roippi

 

[xKiv]

(yes, Java calls them fields)

Huh, when did *that* happen? All through school (IT college) and work (mostly with java), I have only known them as attributes ...

FWIW this is where I learned GC

Hmm. Already in the first paragraph they have "The object will not be reclaimed, leaving the reference dangling" which is makes me cringe (to me, it parses as "GC will not reclaim the object, and therefore the reference will be left dangling" but they want "GC will not reclaim the object, and therefore the reference will be prevented from being left dangling"). I am not sure I would recommend this as learning material, on that basis. (is it translated from chinese [guessing from page title]?)
And "Eligible objects do not affect the future course of program execution." is a "lie to children" (finalizers complicate things, as mentioned later on the page).
They are also mudding the waters by talking about "constituent objects" of an object. Java doesn't have composite objects. Any "constituent objects" are normal objects referenced from a field. Maybe they mean primitive values, but those are not objects.

and you can see how the figure gave me that impression.

[/quote]

I don't. And the first figure already has "eligible" objects with references to "reachable" objects, all reachable objects are reachable because there are references to them from somewhere else ...
But then, I learnt programming on languages without GC (and most of them had pointers).
And most of what I know about java's GC is from oracle's pages about tuning java's GC.

No, that's not how I was using strong reference.

OK, but using it *at all* gave me the wrong impression. Because in java you have to work to get anything weaker.
I was also under the mistaken impression that you used it more than you really did.

 

[roippi]

I'll try that when I get back home and send you a Dropbox-link when it's been uploaded.

Any luck?