GNU bug report logs -
#79023
30.1.90; Suspicion of memory leak on internal_redisplay (MacOS)
Previous Next
Full log
View this message in rfc822 format
On Mon, Aug 11, 2025 at 08:47:44PM +0200, Przemysław Alexander Kamiński wrote:
>
> I'm attaching 6 patches (for easy review/scrutiny/integration).
> Those patches that I marked as tested was used by me for >1 week.
>
> * render_block_small.patch (significant impact, tested):
>
> This is similar patch to previous one, but it's minimal. It's crucial
> to avoid "blink" redraws.
> Still breaks smooth resizing, but frame-resize-pixelwise set to true
> makes it unnoticable.
I'm not convinced this is the way to do this. I still think a better
solution would be to move the redisplay call to the EmacsLayer's
'display' method. As I understand it the system should only call that
method when it actually wants to display the output. Is that called
too often?
> * cache_all_font_results.patch (significant impact, requires testing):
>
> This one prevents huge leak. I was trying to debug emacs-mac version (which
> has very deep traces for allocations, more 256 deep, which is limit for
> Instrumentations).
>
> That led to full malloc logging and discovery that macfont listings were
> allocating plenty of font-related data. Some of it it never cleared.
>
> I didn't test it and worry that caching "null" result might introduce
> some bugs, but it was hundreds of megabytes of allocations on -Q instance.
I can't comment on this as I have no idea how macfont.m works. Perhaps
some of the people working on the Mac port can help.
> * dont_flush_cg.patch (medium impact, tested):
>
> Based on documentation of CGContextFlush it should seldom be used, as
> it's much better to give OS possibility to decide when to flush. It
> prevents non-needed renders (and allocations etc.)
See notes below.
> * misc_releases.patch (small-mid impact, tested):
>
> This one is based on Rudolf Adamkovič patch with some minor extra
> releases and adjustments
> (like releasing an object before setting it to nil ;))
EmacsLayer is only used on Cocoa, and only on certain versions, so
anything that references it needs to be limited. Look at the #if
before the definition of EmacsLayer.
> * ns_native_api_dict.patch (small-mid impact, needs testing):
>
> A small leak, but modified function had ~600_000 allocations after very
> short test runs. Seems like it wasted a lot of power, so I decided it
> should be checking dictionary hydrated at start and queried later
> instead of checking during runtime.
>
> I believe that high NSString allocation amount was indirectly caused by
> using autorelease pools.
LGTM except for the same problem as above. You'll need to look at the
#if or #ifdefs around the code and ensure the new code is correctly
excluded when building on GNUstep.
I think you also removed a #if but left the associated #else and
#endif, although I could be wrong about that.
> * nullify_frame.patch (small impact, tested):
>
> releases reference to a frame, as I've found traces in which resources
> were left behind, small leak.
This one is funny as emacsframe is a pointer to a "struct frame" and
we're not using garbage collection in NS so I don't see how simply
having a reference in a released object can prevent that struct from
being properly freed. Am I missing something?
> + /* Don't reuse inner - cleanup is robust enough */
> + emacsframe = nil;
> +
And I don't understand the comment.
> diff --git a/src/nsterm.m b/src/nsterm.m
> index b006b4d5dd..057d876a35 100644
> --- a/src/nsterm.m
> +++ b/src/nsterm.m
> @@ -9033,7 +9033,6 @@
>
> #if defined (NS_IMPL_COCOA) && MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
> CGContextRef context = [(EmacsLayer *)[self layer] getContext];
> - CGContextFlush (context);
>
> double scale = [[self window] backingScaleFactor];
> int bpp = CGBitmapContextGetBitsPerPixel (context) / 8;
Immediately after this flush we access the pixels directly in the
buffer, so we *require* that all drawing actions in the context have
been completed, otherwise we risk doing actions out of order.
Therefore this flush is required.
> @@ -10994,7 +10993,6 @@
> if (!context)
> return;
>
> - CGContextFlush (context);
> CGContextRelease (context);
> context = NULL;
This one is probably superfluous as the CGContextRelease will probably
do the flush itself, but I don't know that for sure and we definitely
need it flushed before we can perform the next action, so I don't see
this as a problem.
Aside from that I think there was maybe some non-GNU-standard style C
in the nsimage patch, but I'm not entirely clear how it should look
myself.
Thanks!
--
Alan Third
This bug report was last modified today.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.