Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

w3mimgdisplay consumes a lot of memory #451

Closed
soredake opened this issue Jan 16, 2016 · 17 comments
Closed

w3mimgdisplay consumes a lot of memory #451

soredake opened this issue Jan 16, 2016 · 17 comments

Comments

@soredake
Copy link

After scrolling a 20-30 images it consumes up to 2gb+ memory.
1452941014
Related https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=218857

@hut
Copy link
Member

hut commented Jan 18, 2016

Yep, it keeps a cache. I think this should really be fixed in w3m rather than in ranger. :S

@denzerdhard
Copy link

Add -gtk flag to w3m if using gentoo, this fixed it for me.

@soredake
Copy link
Author

soredake commented Feb 4, 2016

Thanks, fixed consumtion.

@soredake soredake closed this as completed Feb 4, 2016
@Ambrevar
Copy link
Contributor

Ambrevar commented Jun 1, 2016

Since I don't have a gentoo at hands just now, I had a look at what the -gtk USE flag does:
https://gitweb.gentoo.org/repo/gentoo.git/tree/www-client/w3m/w3m-0.5.3-r8.ebuild
I tried to compile with the --with-imagelib=gtk2 configure option to no avail. Any suggestion?

@denzerdhard
Copy link

No you need opposite without gtk but with imlib2. Try --with-imagelib=imlib2

@Ambrevar
Copy link
Contributor

That's what I had before, still the same issue.

@Ambrevar
Copy link
Contributor

Tried different options, did not work for me. When only imlib2 is set, it crashes and nothing renders in ranger.
I'm currently running Arch Linux with the following compilation flags:

CPPFLAGS="-D_FORTIFY_SOURCE=2"
CFLAGS="-march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong"
CXXFLAGS="-march=x86-64 -mtune=generic -O2 -pipe -fstack-protector-strong"
LDFLAGS="-Wl,-O1,--sort-common,--as-needed,-z,relro"

@mrogalski
Copy link
Contributor

I'll include the full configure from my gentoo installation. Maybe this will help You somehow?

./configure --prefix=/usr --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --mandir=/usr/share/man --infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc --localstatedir=/var/lib --libdir=/usr/lib64 --with-editor=/usr/bin/vi --with-mailer=/bin/mail --with-browser=/usr/bin/xdg-open --with-termlib=yes --enable-image=x11,fb --with-imagelib=imlib2 --with-migemo=no --enable-m17n --enable-unicode --enable-mouse --enable-nls --disable-nntp --enable-digest-auth --with-ssl --disable-xface --enable-japanese=U --enable-keymap=w3m

@mrogalski
Copy link
Contributor

Oh, and BTW, from man 7 feature_test_macros:

With _FORTIFY_SOURCE set to 2, some more checking is added, but some conforming programs might fail.

@Ambrevar
Copy link
Contributor

Hmm... Tried your exact same 'configure' (although I've never really trusted autoconf), and _FORTIFY_SOURCE=1, I am still experiencing the same leak. Problem might be with imlib then. Or something else.

@soredake soredake reopened this Dec 17, 2016
@nfnty
Copy link
Member

nfnty commented Jan 25, 2017

These issues should be reported upstream in w3m.

@nfnty nfnty closed this as completed Jan 25, 2017
@nfnty nfnty removed the enhancement label Jan 25, 2017
@afarah1
Copy link

afarah1 commented Jun 20, 2017

Well this is very easy to fix on ranger... Here's a patch (for ext/img_display.py from ranger-stable 1.8.1). You just kill w3mimgdisplay after drawing the image and fork it again when drawing another... The delay between drawing an image and another is the same for me with the current version of ranger and with the patch. There is no noticeable overhead from the fork on my system.

This is kind of a workaround, so how to fix it on w3m... If anyone is interested I report an attempt below, with no success.

@mrogalski I'm assuming you're using the latest version of w3m (0.5.3 from 2011)? I'm on Gentoo too and I have the same useflags that you except for l10n_ja, the Japanese one, but I still have this issue (without the supplied patch). Here is a diff from my ./configure to yours:

./configure --prefix=/usr --build=x86_64-pc-linux-gnu --host=x86_64-pc-linux-gnu --mandir=/usr/share/man --infodir=/usr/share/info --datadir=/usr/share --sysconfdir=/etc --localstatedir=/var/lib [---docdir=/usr/share/doc/w3m-0.5.3-r9 --htmldir=/usr/share/doc/w3m-0.5.3-r9/html-] --libdir=/usr/lib64 --with-editor=/usr/bin/vi --with-mailer=/bin/mail --with-browser=/usr/bin/xdg-open --with-termlib=yes [---enable-image=x11-] {+--enable-image=x11,fb+} --with-imagelib=imlib2 [---without-migemo-] {+--with-migemo=no+} --enable-m17n --enable-unicode [---disable-mouse-] {+--enable-mouse+} --enable-nls --disable-nntp --enable-digest-auth --with-ssl --disable-xface [---with-charset=UTF-8-] {+--enable-japanese=U+} --enable-keymap=w3m

Notice that besides the Japanese stuff (migemo is related to that) and the disabling of mouse support the sole difference is {+--enable-image=x11,fb+}. Even though x11 seems to be used in the end I got the tarball from Portage (w3m-0.5.3-git20161120.tar.gz) and built it manually, with --enable-image=x11 (without fb support) just like you have, to no avail. Still keeps consuming memory until the OOM killer gets to it.

There are several entries on w3m's Changelog related to image caching. The code is not very well documented, so I recompiled w3m with debug info and modified ranger to run massif when calling w3mimgdisplay:

        self.process = Popen(['/usr/bin/valgrind', '--tool=massif', self.binary_path] + W3MIMGDISPLAY_OPTIONS,
                stdin=PIPE, stdout=PIPE, universal_newlines=True)

Here's a profile. Of course one can run w3mimgdisplay directly with valgrind, passing the correct format strings, but ranger does that for us. Anyway, I'm not sure if the issue is a memory leak or if it's intentional caching... I tried to fix memory leaks but that didn't help. I only report it so other people don't waste time as well:

It seems that x11_w3mimg is opening an X display but never closing it (see the profile above). I tried implementing x11_close (which is commented out) and also calling XCloseDisplay at the end of w3mimg_x11open which was just calling free, but that wasn't the issue. So I just ran valgrind without massif to look at memory leaks:

        self.process = Popen(['valgrind', '--log-file=val.%p', '--leak-check=full', '--show-leak-kinds=all', self.binary_path] + W3MIMGDISPLAY_OPTIONS,
                stdin=PIPE, stdout=PIPE, universal_newlines=True)

It seems that the relevant leak occurs at:

==13149==    by 0x402C6E: x11_load_image (x11_w3mimg.c:431)                     
==13149==    by 0x40229B: DrawImage (w3mimgdisplay.c:296)                       
==13149==    by 0x4017B4: main (w3mimgdisplay.c:124)   

See here for the full output. Though imlib_free_image is being called... I thought it was a missing w_op->free_image on DrawImage but that didn't seem to solve the issue either... So yeah, I'm happy to stay with my ranger patch...

@dquinton
Copy link

I too am making RAM cakes with w3m... dir of 50 images eats 1.5 Gig

I'm trying to apply the patch suggested by afarah1, but I'm not sure how to insert the "self.quit()" line

are we looking at the lines in ext/img_display.py

def initialize(self):
        """start w3mimgdisplay"""
        self.binary_path = None
        self.binary_path = self._find_w3mimgdisplay_executable()  # may crash
        self.process = Popen([self.binary_path] + W3MIMGDISPLAY_OPTIONS,
                             stdin=PIPE, stdout=PIPE, universal_newlines=True)
        self.is_initialized = True

? where do I insert the patch?
Ranger 1.9.0b5

@soredake
Copy link
Author

soredake commented Aug 28, 2017

--- ranger/ext/img_display.py.old	2017-08-28 15:57:15.651869415 +0300
+++ ranger/ext/img_display.py	2017-08-28 15:59:41.258670713 +0300
@@ -106,6 +106,8 @@
             start_y, width, height))
         self.process.stdin.flush()
         self.process.stdout.readline()
+        self.quit()
+        self.is_initialized = False
 
     def clear(self, start_x, start_y, width, height):
         if not self.is_initialized or self.process.poll() is not None:

for 1.8.1

@afarah1
Copy link

afarah1 commented Sep 2, 2017

@dquinton just add the two lines at the end of the draw function as indicated by soredake.

@pirate486743186
Copy link
Contributor

Can't you guys use the above patch?

Hacks to work around bugs in the libraries are ok. As long as you keep them well documented and easily removable.

Maybe have a runtime/config option to activate them. I see quite a few "not our bug"

@toonn
Copy link
Member

toonn commented Aug 17, 2021

PRs will be considered. We'd really rather prefer you switch to ueberzug though. That method has proved a lot more stable than w3mimgpreview.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

11 participants