Re: memory leak - cairo/x11 backend
On 22.12.2013 19:58, Riccardo Mottola wrote:
> Fred Kiefer wrote:
>> I am able to reproduce this behaviour, but I am unsure about what it
>> means. I did an in depth inspection with the memory panel of GNUstep,
>> but this didn't show up a specific issue. (BTW the memory panel is hard
>> to use with Laterna Magica. The AppController registers itself for the
>> notification NSTableViewSelectionDidChangeNotification and as the
>> fileListView is still null at the point in time where this happens it
>> will get called even from the memory panel. The same is true for the
>> other notification. You should move these calls into the awakeFromNib
>> method)
> Fine, I just moved those calls as you suggested.
No you didn't :-(
> I pointed out to two memory leaks: not only the Xorg doesn't get back to
> its original memory usage, but also LaternaMagica itself: in the example
> I went from X: 108 LM: 58, to X: 142 LM: 91. That is about a 35MB
> increase for the server, but also a 33MB increase of LM! Thus I see a
> problem on both sides, let's analyze LM, perhaps the problem is there
> and now the Xorg problem is a reflection of that.
>
> You didn't find anything obvious, I too tried the memory panel without
> success. At start, I see no NSImageViews! why?
>
> Clicking on an item yields
> 2013-12-22 18:47:19.268 LaternaMagica[14382] Problem posting
> notification: NAME:NSRangeException
> REASON:Index 11 is out of range 0 (in
> 'objectAtIndex:') INFO:{Array = (); Count = 0; Index = 11; }
You get this message as I explained above because the AppController
tries to handle notifications from the memory panel. Move the calls and
this will stop to show up. If it doesn't, then the instance variable
fileListView you are using is not set during NIB loading. Please check
your Gorm file in that case.
> Anyway, at start I see:
> NSBitmapImageRep: 12
> NSImageView: ?
> NSImage: ?
You should at least open the open file panel once to have a real
baseline. Better yet, open one image and remove it and use that
situation for the memory panel.
> I load an image, while displaying it:
> NSBitmapImageRep: 34
> NSImageView: 1
> NSImage: 193
>
> I remove the image (do not "delete" it while testing :) )
> NSBitmapImageRep: 33
> NSImageView: 1
> NSImage: 192
>
> I don't know the original state, but it went back of one, so Indeed
> something get released, but is it enough?
You should keep on adding and removing an image and look at the
difference between before and after. If there is a leak in GNUstep
objects and you do these steps often enough the class will show up in
the memory panel.
If we have a leak at C level or in the X resource consumption you wont
find it that way.
>> There is a lot of other strange code in Laterna Magica, for example:
>>
>>[view setImage: destImage];
>>[view setNeedsDisplay:YES];
>>[[view superview] setNeedsDisplay:YES];
>>[window displayIfNeeded];
>>
>> Here only the first line should be needed. Why is the rest there?
>> But none of that looks like an obvious memory leak.
> I will check that, it looks I desperately had some redraw problems. That
> code needs to work both inside a window and with a full-screen window.
> Perhaps it is not needed anymore, I will check on Mac and GNUstep.
> However, you say yourself that it shouldn't cause memory problems. At
> most, it is inefficient/inelegant.
>
>>
>> I also tried to see any specific memory leaks with valgrind, but here
>> the normal GNUstep leaks overshadow the output.
> Well, "normal leaks".
I was referring to valgrind reporting any font or image that GNustep
loads and stores in global variables as a leak. Richard added code to
base to be able to remove such resources at program exit to be able to
inspect memory issues better. Maybe I need to add such code in gui and
back as well.
>> And I am still not completely convinced that there is a problem. When I
>> keep on loading and removing images the size of the XOrg process keeps
>> returning to the same value it had before. Not to the initial one, but
>> it does not keep on growing endlessly. It is more like the biggest image
>> ever loaded in Laterna Magica determines the additional resource usage
>> of the X process.
You did not reply to this bit. I think it is the most important one. If
we are really talking about a leak it should get worse, but this is not
what I see. When I keep on loading one image and remove it immediately
the memory consumption stays at about the same level. When I load a lot
of images at once the value goes up and stays there.
___
Gnustep-dev mailing list
Gnustep-dev@gnu.org
https://lists.gnu.org/mailman/listinfo/gnustep-dev
Re: memory leak - cairo/x11 backend
Hi,
Thanks for taking a look at it.
Fred Kiefer wrote:
I am able to reproduce this behaviour, but I am unsure about what it
means. I did an in depth inspection with the memory panel of GNUstep,
but this didn't show up a specific issue. (BTW the memory panel is hard
to use with Laterna Magica. The AppController registers itself for the
notification NSTableViewSelectionDidChangeNotification and as the
fileListView is still null at the point in time where this happens it
will get called even from the memory panel. The same is true for the
other notification. You should move these calls into the awakeFromNib
method)
Fine, I just moved those calls as you suggested.
I pointed out to two memory leaks: not only the Xorg doesn't get back to
its original memory usage, but also LaternaMagica itself: in the example
I went from X: 108 LM: 58, to X: 142 LM: 91. That is about a 35MB
increase for the server, but also a 33MB increase of LM! Thus I see a
problem on both sides, let's analyze LM, perhaps the problem is there
and now the Xorg problem is a reflection of that.
You didn't find anything obvious, I too tried the memory panel without
success. At start, I see no NSImageViews! why?
Clicking on an item yields
2013-12-22 18:47:19.268 LaternaMagica[14382] Problem posting
notification: NAME:NSRangeException
REASON:Index 11 is out of range 0 (in
'objectAtIndex:') INFO:{Array = (); Count = 0; Index = 11; }
Anyway, at start I see:
NSBitmapImageRep: 12
NSImageView: ?
NSImage: ?
I load an image, while displaying it:
NSBitmapImageRep: 34
NSImageView: 1
NSImage: 193
I remove the image (do not "delete" it while testing :) )
NSBitmapImageRep: 33
NSImageView: 1
NSImage: 192
I don't know the original state, but it went back of one, so Indeed
something get released, but is it enough?
There is a lot of other strange code in Laterna Magica, for example:
[view setImage: destImage];
[view setNeedsDisplay:YES];
[[view superview] setNeedsDisplay:YES];
[window displayIfNeeded];
Here only the first line should be needed. Why is the rest there?
But none of that looks like an obvious memory leak.
I will check that, it looks I desperately had some redraw problems. That
code needs to work both inside a window and with a full-screen window.
Perhaps it is not needed anymore, I will check on Mac and GNUstep.
However, you say yourself that it shouldn't cause memory problems. At
most, it is inefficient/inelegant.
I also tried to see any specific memory leaks with valgrind, but here
the normal GNUstep leaks overshadow the output.
Well, "normal leaks".
And I am still not completely convinced that there is a problem. When I
keep on loading and removing images the size of the XOrg process keeps
returning to the same value it had before. Not to the initial one, but
it does not keep on growing endlessly. It is more like the biggest image
ever loaded in Laterna Magica determines the additional resource usage
of the X process.
Fred
Riccardo
___
Gnustep-dev mailing list
Gnustep-dev@gnu.org
https://lists.gnu.org/mailman/listinfo/gnustep-dev
Re: memory leak - cairo/x11 backend
I am able to reproduce this behaviour, but I am unsure about what it means. I did an in depth inspection with the memory panel of GNUstep, but this didn't show up a specific issue. (BTW the memory panel is hard to use with Laterna Magica. The AppController registers itself for the notification NSTableViewSelectionDidChangeNotification and as the fileListView is still null at the point in time where this happens it will get called even from the memory panel. The same is true for the other notification. You should move these calls into the awakeFromNib method) There is a lot of other strange code in Laterna Magica, for example: [view setImage: destImage]; [view setNeedsDisplay:YES]; [[view superview] setNeedsDisplay:YES]; [window displayIfNeeded]; Here only the first line should be needed. Why is the rest there? But none of that looks like an obvious memory leak. I also tried to see any specific memory leaks with valgrind, but here the normal GNUstep leaks overshadow the output. And I am still not completely convinced that there is a problem. When I keep on loading and removing images the size of the XOrg process keeps returning to the same value it had before. Not to the initial one, but it does not keep on growing endlessly. It is more like the biggest image ever loaded in Laterna Magica determines the additional resource usage of the X process. Fred On 20.12.2013 15:46, Riccardo Mottola wrote: > Hi developers, > > thanks for looking. I have seen Fred patched your patch, now I tested > everything, on cairo for now. > > I tested everything before and after. > > Task: load two images. Check memory of Xorg and LM > 1) before > then load two images in LM > 2) displaying the last image (automatically selected > 3) displaying the butlast image (changing image) > 4) removing them again. through the Clear menu entry > > Before your patch: > > 1) X:112 LM:58 > 2) X: 132 LM: 76 > 3) X: 152 LM: 93 > 4) X: 150 LM:91 > > After your patch: > 1) X: 108 LM: 58 > 2) X: 127 LM: 76 > 3) X: 144 LM: 93 > 4) X: 142 LM: 91 > > I notice that we consume less memory, that's fine! > > However still after I do "remove all pictures" in LaternaMagica and > nothing gets displayed, LaternaMagica and X get not back to the initial > memory usage. Perhaps there is a leak in LM I am not finding? that would > also explain why the X server doesn't free all the memory. > > > Riccardo > ___ Gnustep-dev mailing list Gnustep-dev@gnu.org https://lists.gnu.org/mailman/listinfo/gnustep-dev
Re: memory leak - cairo/x11 backend
Hi developers, thanks for looking. I have seen Fred patched your patch, now I tested everything, on cairo for now. I tested everything before and after. Task: load two images. Check memory of Xorg and LM 1) before then load two images in LM 2) displaying the last image (automatically selected 3) displaying the butlast image (changing image) 4) removing them again. through the Clear menu entry Before your patch: 1) X:112 LM:58 2) X: 132 LM: 76 3) X: 152 LM: 93 4) X: 150 LM:91 After your patch: 1) X: 108 LM: 58 2) X: 127 LM: 76 3) X: 144 LM: 93 4) X: 142 LM: 91 I notice that we consume less memory, that's fine! However still after I do "remove all pictures" in LaternaMagica and nothing gets displayed, LaternaMagica and X get not back to the initial memory usage. Perhaps there is a leak in LM I am not finding? that would also explain why the X server doesn't free all the memory. Riccardo ___ Gnustep-dev mailing list Gnustep-dev@gnu.org https://lists.gnu.org/mailman/listinfo/gnustep-dev
Re: memory leak - cairo/x11 backend
On 18.12.2013 18:45, Eric Wasylishen wrote:
> Hi Riccardo, +Fred +gnustep-dev
>
> On Dec 18, 2013, at 7:40 AM, Riccardo Mottola
> wrote:
>
>> Hi,
>>
>> Eric Wasylishen wrote:
>>>
>>> Hi Riccardo,
>>>
>>> As a starting point for debugging I would put NSLog's in
>>> XGCairoModernSurface -initWithDevice: and -dealloc. This is where
>>> we retain / release the Cairo surface that is holding memory in
>>> the x server.
>>>
>>> Also I would put logging in -NSImage set name:, log [nameDict
>>> allKeys] to see which images are kept in memory by nameDict.
>>>
>> I did so, in setName.. It does not get called from laternaMagica
>> when switching images. WHile the application loads and shows its
>> windows I see stuff like:
>>
>> 2013-12-18 15:36:39.356 LaternaMagica[18906] NSimage setName, keys:
>> ("common_ret H", "common_Nibble", "common_HomeDirectory",
>> NSApplicationIcon, "common_UnknownT ool", GSMenuArrow,
>> "common_Mount", "common_DownloadFolder", "common_ArrowDown",
>> "laternamagica_48.tif", "common_Home", "common_ArrowLeftH",
>> "common_Close", "com mon_3DArrowLeft", "common_Desktop",
>> "common_ImageFolder", GSMenuSelected, "commo n_3DArrowRightH",
>> GSSwitch, GSSwitchSelected, "common_GSFolder", "common_Dimple" ,
>> "common_Unmount", "common_DimpleHoriz", "common_ArrowRightH",
>> "common_DocsFold er", "common_Unknown", "common_ArrowUpH",
>> GSMenuMixed, NSFolder, "common_ArrowUp ", "common_3DArrowUp",
>> "common_ArrowRight", "common_3DArrowDown", "common_ArrowD ownH",
>> "common_ret", "common_ArrowLeft", "common_SliderHoriz")
>>
>>
>> however when I load images from disk and cycle between them,
>> nothing gets printed, meaning that setName doesn't get called.
>>
>> Riccardo
>
> Actually, I was on the wrong track; I just found the problem.
>
> During the creation of a window, we run -[XGServerWindow
> setWindowdevice:forContext:]. The problem is, inside that method we
> call [self _createBuffer] which does window->buffer =
> XCreatePixmap(…), but the [self _createBuffer] call is *before*
> window->gdriverProtocol is initialized. window->gdriverProtocol is
> set from the -init… method of XGCairoModernSurface.
>
> The call sequence that causes the leak looks like:
>
> -[XGServerWindow setWindowdevice:forContext:] -[XGServerWindow
> _createBuffer] (performs window->buffer = XCreatePixmap because
> window->gdriverProtocol == 0) GSSetDevice() -[XGCairoModernSurface
> initWIthDevice:] (sets window->gdriverProtocol to
> GDriverHandlesExpose | GDriverHandlesBacking) …later….
> -[XGServerWindow termwindow] doesn’t free window->buffer because
> window->gdriverProtocol has the GDriverHandlesBacking bit set.
>
>
>
> As a quick fix, I reordered this section of -[XGServerWindow
> setWindowdevice:forContext:]
>
> if (window->buffer == 0) { [self _createBuffer: window]; }
>
> [self styleoffsets: &l : &r : &t : &b :
> window->win_attrs.window_style : window->ident]; GSSetDevice(ctxt,
> window, l, NSHeight(window->xframe) + b);
>
> to:
>
> [self styleoffsets: &l : &r : &t : &b :
> window->win_attrs.window_style : window->ident]; GSSetDevice(ctxt,
> window, l, NSHeight(window->xframe) + b); if (window->buffer == 0) {
> [self _createBuffer: window]; }
>
>
> which fixed the leak, however it causes apps to segfault with the
> xlib backend. Probably some code in -styleoffsets or GSSetDevice
> attempts to use window->buffer.
>
> I just committed a more foolproof fix, which is just disabling all of
> the code in -_createBuffer if we are building with the cairo backend.
> As I noted in the comment there, I don’t like the
> window->gdriverProtocol abstraction as it’s trying to handle
> different library configurations in “too dynamic” of a way.
>
> Testing is welcome; the leak is gone for me and LaternaMagica seems a
> lot faster too.
>
> Cheers, Eric
Excellent, you found the source of the issue and fixed it. I agree that
the fix doesn't look very nice. I will think hard to come up with a
cleaner way to achieve the same.
Thank you and also a thank you to Riccardo for noticing this issue,
Fred
___
Gnustep-dev mailing list
Gnustep-dev@gnu.org
https://lists.gnu.org/mailman/listinfo/gnustep-dev
Re: memory leak - cairo/x11 backend
Hi Riccardo, +Fred +gnustep-dev
On Dec 18, 2013, at 7:40 AM, Riccardo Mottola
wrote:
> Hi,
>
> Eric Wasylishen wrote:
>>
>> Hi Riccardo,
>>
>> As a starting point for debugging I would put NSLog's in
>> XGCairoModernSurface -initWithDevice: and -dealloc. This is where we retain
>> / release the Cairo surface that is holding memory in the x server.
>>
>> Also I would put logging in -NSImage set name:, log [nameDict allKeys] to
>> see which images are kept in memory by nameDict.
>>
> I did so, in setName.. It does not get called from laternaMagica when
> switching images. WHile the application loads and shows its windows I see
> stuff like:
>
> 2013-12-18 15:36:39.356 LaternaMagica[18906] NSimage setName, keys:
> ("common_ret H", "common_Nibble", "common_HomeDirectory", NSApplicationIcon,
> "common_UnknownT ool", GSMenuArrow, "common_Mount", "common_DownloadFolder",
> "common_ArrowDown", "laternamagica_48.tif", "common_Home",
> "common_ArrowLeftH", "common_Close", "com mon_3DArrowLeft", "common_Desktop",
> "common_ImageFolder", GSMenuSelected, "commo n_3DArrowRightH", GSSwitch,
> GSSwitchSelected, "common_GSFolder", "common_Dimple" , "common_Unmount",
> "common_DimpleHoriz", "common_ArrowRightH", "common_DocsFold er",
> "common_Unknown", "common_ArrowUpH", GSMenuMixed, NSFolder, "common_ArrowUp
> ", "common_3DArrowUp", "common_ArrowRight", "common_3DArrowDown",
> "common_ArrowD ownH", "common_ret", "common_ArrowLeft", "common_SliderHoriz")
>
>
> however when I load images from disk and cycle between them, nothing gets
> printed, meaning that setName doesn't get called.
>
> Riccardo
Actually, I was on the wrong track; I just found the problem.
During the creation of a window, we run -[XGServerWindow
setWindowdevice:forContext:]. The problem is, inside that method we call [self
_createBuffer] which does window->buffer = XCreatePixmap(…), but the [self
_createBuffer] call is *before* window->gdriverProtocol is initialized.
window->gdriverProtocol is set from the -init… method of XGCairoModernSurface.
The call sequence that causes the leak looks like:
-[XGServerWindow setWindowdevice:forContext:]
-[XGServerWindow _createBuffer]
(performs window->buffer = XCreatePixmap because window->gdriverProtocol == 0)
GSSetDevice()
-[XGCairoModernSurface initWIthDevice:]
(sets window->gdriverProtocol to GDriverHandlesExpose | GDriverHandlesBacking)
…later….
-[XGServerWindow termwindow] doesn’t free window->buffer because
window->gdriverProtocol has the GDriverHandlesBacking bit set.
As a quick fix, I reordered this section of -[XGServerWindow
setWindowdevice:forContext:]
if (window->buffer == 0)
{
[self _createBuffer: window];
}
[self styleoffsets: &l : &r : &t : &b
: window->win_attrs.window_style : window->ident];
GSSetDevice(ctxt, window, l, NSHeight(window->xframe) + b);
to:
[self styleoffsets: &l : &r : &t : &b
: window->win_attrs.window_style : window->ident];
GSSetDevice(ctxt, window, l, NSHeight(window->xframe) + b);
if (window->buffer == 0)
{
[self _createBuffer: window];
}
which fixed the leak, however it causes apps to segfault with the xlib backend.
Probably some code in -styleoffsets or GSSetDevice attempts to use
window->buffer.
I just committed a more foolproof fix, which is just disabling all of the code
in -_createBuffer if we are building with the cairo backend. As I noted in the
comment there, I don’t like the window->gdriverProtocol abstraction as it’s
trying to handle different library configurations in “too dynamic” of a way.
Testing is welcome; the leak is gone for me and LaternaMagica seems a lot
faster too.
Cheers,
Eric
PS, ignore my comments about caching, for some reason I was thinking that all
NSImages would go in namedict. What we have now is more or less fine, UI
resources loaded with +imageNamed: are kept cached in nameDict and others are
not.
___
Gnustep-dev mailing list
Gnustep-dev@gnu.org
https://lists.gnu.org/mailman/listinfo/gnustep-dev
