- Dillo-dev - lists.mailman3.com

[patch] dw2 memory reduction

by jcid＠dillo.org

On Sun, May 25, 2008 at 06:47:11PM +0200, Johannes Hofmann wrote: > On Sun, May 25, 2008 at 09:12:11AM -0400, Jorge Arellano Cid wrote: > > On Sun, May 25, 2008 at 10:08:12AM +0200, Johannes Hofmann wrote: > > > Hi, > > > > > > On Sat, May 24, 2008 at 04:39:42PM -0400, Jorge Arellano Cid wrote: > > > > On Sat, May 24, 2008 at 02:58:08PM +0200, Johannes Hofmann wrote: > > > > > Hi, > > > > > > > > > > attached is patch to reduce the memory usage of dw2: > > > > > > > > Excellent! > > > > (AFAIU this is near 30% memory usage reduction in dw2) > > > > > > > > Committed. > > > > > > > > > > > > > > * Styles are now reused if they are equal. This brings down the > > > > > number of Style objects on the mysql page from around 180000 to > > > > > 23000 and reduces memory usage by about 25M at the cost of an > > > > > additional HashTable. > > > > > Maybe the hash table should be part of fltkplatform > > > > > as the hash tables for Color and Font? > > > > > There would be even more sharing of styles possible, if > > > > > x_link would not be part of style. On the other hand about 3M for > > > > > styles on the huge mysql page is pretty ok. > > > > > > > > > > * Size of struct Word is reduced by 4 integers by using shorts. I > > > > > know that this is questionable, but I think in the case of struct > > > > > Word that can be allocated a lot it is worth the hassle. > > > > > Please check whether my assumption that space lengths and word > > > > > lengths can be stored in shorts are valid in general. > > > > > > > > It looks OK at first sight, but to be sure it'd be good to know > > > > what happens when the limit is surpassed. > > > > > > Just tried it on a page with 40000 char long word. With short for > > > hlStart/hlEnd, selection is - obviously - no longer possible. > > > That's clearly a regression. > > > Please revert with attached patch. > > > > A max of 32K for a single word, looks quite reasonable to me. > > I meant, if the browser doesn't crash, and fails gracefully, why > > not. > > > > BTW, it can be extended to 64K with unsigned, by using 65535 as > > sentinel value (instead of -1), but again, is it worth? > > > > It'd be good to know how much "struct Word" is reduced in size > > (after compiler alignment), and how much it impacts overall > > memory usage. I mean if struct word is reduced 20% and that > > yields a 4% overall memory reduction, it may be not worth > > complicating the data type. > > > > OTOH, ... > > On the very long mysql page, addWord is called 1192256 times. But > due to the allocation in 2^n steps in SimpleVector memory for > some more of them could be allocated > structs on 32bit systems are 8 byte aligned. struct Word with > highlight stuff as int is 48 byte. With short it's 40 byte. > On the mysql page I observe a total memory reduction of about 9M by > using short's for the highlight stuff. > > Apart from the highlight stuff dillo can handle those long words > just fine. So we would add this restriction just to save 9M of > memory on this rather extreme page. > > I would keep int's for now and try to get hlStart and hlEnd out of > struct Word and in Textblock which would bring down struct Word to > 32 bytes - hopefully without any restrictions. OK, patch reverted. Sorry for asking for this important but boring-to-collect data. :) Let's see, if it was 250MB with the old scheme, down to 225M with the style reuse, these shorts are roughly a 4% memory reduction. -- Cheers Jorge.-

May 25, 2008

Fuzzing dillo

by corvid＠lavabit.com

Justus wrote: > Jorge Arellano Cid wrote: > > The problem of > > being robust when parsing garbage or malicious code needs a > > general strategy. Certainly testing flags all the time is very inadequate when the spec has it as, for example, <!ELEMENT SELECT - - (OPTGROUP|OPTION)+ -- option selector --> (i.e., don't put anything in a select except optgroup or option)... > In case these bug reports are useful at this point, here is another one: > > In file html.cc, function Html_tag_close_select: > > int size = input->select->options->size (); > > fails since input->select is NULL. The html fragment that triggers this > fault is attached. ..but since flags are what there is at the moment, here's a patch.

May 25, 2008

Q: why does Nav_stack_clean() free the data and Nav_stack_truncate() not?

by jcid＠dillo.org

On Sun, May 25, 2008 at 04:39:46PM +0100, Jeremy Henty wrote: > On Sun, May 25, 2008 at 09:31:30AM -0400, Jorge Arellano Cid wrote: > > > Hi Jeremy, > > > > Nav_stack_truncate() and Nav_stack_insert() are ad-hoc > > bindings. They're called just in one place of the code. They're > > not clean well-defined functions of an ADT. > > > > This is "your" mistake. :-) > > I'll cheerfully abandon my expectations of consistency. :-) But what > about memory[1]? Doesn't it leak when Nav_stack_truncate() shrinks > bw->nav_stack ? It seems to me that Nav_stack_truncate() should free > all those pointers (and therefore Nav_stack_insert() shouldn't free > anything). I'll supply a patch if you agree. Yes it leaks. (a_Nav_free is incomplete BTW). Patch committed, plus the 's/insert/append/' patch. > > The nav.c module is asking for a clean ADT, and if you can work on > > it, it's certainly welcomed. > > Then I hope people will review the patch I sent to the discussion with > corvid about the Redirect/Reload crash bug, which is all about the > handling of the navigation stack. > > > BTW, if it's to be rewritten it'd be good to have the TABs > > interface in mind. > > Oh noes, you have spotted my hidden agenda! :-) I was indeed thinking > that if I got my head around this it would prepare me for looking at > tabs. > > > This is, if we end with a clean ADT or object, we could have an > > array of them, one for each tab, and switch freely among navigation > > stacks. > > As a first step, maybe we could modify uicmd.cc to indirect though a > new BrowserWindowSet object. The callbacks would be changed to look > up the current BrowserWindow of the BrowserWindowSet . Initially the > BrowserWindowSet could be a stub that managed a single BrowserWindow , > later it could wrap an array of BrowserWindows . It can be done in several ways. The design needs to consider at least front/background TAB loading and async events. e.g. idle functions, cache updates, messages. -- Cheers Jorge.-

May 25, 2008

patch: http_proxyuser preference

by corvid＠lavabit.com

After I sent the preferences mail yesterday, I found that I overlooked one. The headers look good, but I have no way to test it for real.

May 25, 2008

remaining preferences

by Johannes.Hofmann＠gmx.de

On Sun, May 25, 2008 at 10:51:47AM -0400, Jorge Arellano Cid wrote: <snip> > > Somebody using ION may check this. > Works ok as it is. The dialog windows are placed over the bottom of the main window. A bit like the new search bar. A quite nice concept actually. So I think the preference option can safely be removed. Cheers, Johannes

May 25, 2008

Fuzzing dillo

by 4winter＠informatik.uni-hamburg.de

-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Jorge Arellano Cid wrote: > On Sun, May 25, 2008 at 01:12:38PM +0200, Justus Winter wrote: >> -----BEGIN PGP SIGNED MESSAGE----- >> Hash: SHA1 >> >> Hi :) >> >> I was playing around with Michal Zalewskis html fuzzer 'mangle' trying >> to crash dillo and succeeded :). > > Good. > > It's no surprise because dillo2 needs a careful review of > behaviour when facing strange/malicious values. Obviously this > phase was procrastinated until basic functionality was completed. > > For instance, most probably you'll get to crash dillo2 by > passing it some negative values in attributes. The problem of > being robust when parsing garbage or malicious code needs a > general strategy. Are you saying that it is too early to locate problems this way? The process of finding problems using a fuzzer and generating minimal testcases that trigger the problem is mostly automatic and I could script the last bits that do require manual intervention with ease. In case these bug reports are useful at this point, here is another one: In file html.cc, function Html_tag_close_select: int size = input->select->options->size (); fails since input->select is NULL. The html fragment that triggers this fault is attached. Justus - -- gpg key fingerprint: C82D 382A AB38 1A54 5290 19D6 A0F9 B035 686C 6996 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.6 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFIOayFoPmwNWhsaZYRArZgAJ4hdzV2Gs2fVNRgblDASKmEmaF0gACfaKBT XpPVTVS8l4RITppTqprbAlM= =/sCO -----END PGP SIGNATURE-----

May 25, 2008

remaining preferences

by Johannes.Hofmann＠gmx.de

On Sun, May 25, 2008 at 10:51:47AM -0400, Jorge Arellano Cid wrote: <snip> > > > # Enabling this will restrain OpenUrl and FindText, but may be required > > # for the ION window manager. > > transient_dialogs > > > > from fltk::Window::child_of(): > > Tell the system that this window will not have an icon, it will > > dissappear and reappear when \a parent is iconized or shown, and it > > is forced to always be above \a parent. On X this is called a > > "Transient window", and Windows calls this a "overlapping child". \a > > parent is different than the parent(), which must be zero. > > (and down in fltk::CreatedWindow::create, there's a > > if (window->child_of() && window->child_of()->shown()) > > XSetTransientForHint(xdisplay, x->xid, window->child_of()->i->xid);) > > > > If Justus gets rid of the dialogs, obviously the pref can be removed. > > Enabling this means making our own dialogs for message(), choice(), input(). > > Somebody using ION may check this. Just searched my email. I had requested that feature some time ago - well, it was Nov 2001 ;-) I no longer use ion, but I will check how it works with current dillo. The file dialogs are the only that will stay, right? Cheers, Johannes

May 25, 2008

[patch] dw2 memory reduction

by Johannes.Hofmann＠gmx.de

On Sun, May 25, 2008 at 09:12:11AM -0400, Jorge Arellano Cid wrote: > On Sun, May 25, 2008 at 10:08:12AM +0200, Johannes Hofmann wrote: > > Hi, > > > > On Sat, May 24, 2008 at 04:39:42PM -0400, Jorge Arellano Cid wrote: > > > On Sat, May 24, 2008 at 02:58:08PM +0200, Johannes Hofmann wrote: > > > > Hi, > > > > > > > > attached is patch to reduce the memory usage of dw2: > > > > > > Excellent! > > > (AFAIU this is near 30% memory usage reduction in dw2) > > > > > > Committed. > > > > > > > > > > > * Styles are now reused if they are equal. This brings down the > > > > number of Style objects on the mysql page from around 180000 to > > > > 23000 and reduces memory usage by about 25M at the cost of an > > > > additional HashTable. > > > > Maybe the hash table should be part of fltkplatform > > > > as the hash tables for Color and Font? > > > > There would be even more sharing of styles possible, if > > > > x_link would not be part of style. On the other hand about 3M for > > > > styles on the huge mysql page is pretty ok. > > > > > > > > * Size of struct Word is reduced by 4 integers by using shorts. I > > > > know that this is questionable, but I think in the case of struct > > > > Word that can be allocated a lot it is worth the hassle. > > > > Please check whether my assumption that space lengths and word > > > > lengths can be stored in shorts are valid in general. > > > > > > It looks OK at first sight, but to be sure it'd be good to know > > > what happens when the limit is surpassed. > > > > Just tried it on a page with 40000 char long word. With short for > > hlStart/hlEnd, selection is - obviously - no longer possible. > > That's clearly a regression. > > Please revert with attached patch. > > A max of 32K for a single word, looks quite reasonable to me. > I meant, if the browser doesn't crash, and fails gracefully, why > not. > > BTW, it can be extended to 64K with unsigned, by using 65535 as > sentinel value (instead of -1), but again, is it worth? > > It'd be good to know how much "struct Word" is reduced in size > (after compiler alignment), and how much it impacts overall > memory usage. I mean if struct word is reduced 20% and that > yields a 4% overall memory reduction, it may be not worth > complicating the data type. > > OTOH, ... On the very long mysql page, addWord is called 1192256 times. But due to the allocation in 2^n steps in SimpleVector memory for some more of them could be allocated structs on 32bit systems are 8 byte aligned. struct Word with highlight stuff as int is 48 byte. With short it's 40 byte. On the mysql page I observe a total memory reduction of about 9M by using short's for the highlight stuff. Apart from the highlight stuff dillo can handle those long words just fine. So we would add this restriction just to save 9M of memory on this rather extreme page. I would keep int's for now and try to get hlStart and hlEnd out of struct Word and in Textblock which would bring down struct Word to 32 bytes - hopefully without any restrictions. Cheers, Johannes

May 25, 2008

Fuzzing dillo

by onepoint＠starurchin.org

On Sun, May 25, 2008 at 10:31:23AM -0400, Jorge Arellano Cid wrote: > On Sun, May 25, 2008 at 01:12:38PM +0200, Justus Winter wrote: > > Furthermore I'd suggest to break down html.cc into several files. > > The hard part is a good choice on what to separate. ;) I suggest splitting out everything that handles forms and form elements. That's a fair amount of code that represents a reasonably self-contained chunk. The forms code can also be be cleaned up in a few ways. There's a lot of cut-and-paste code like "foo->get (foo->size () - 1)". Adding getLast() methods to some lout classes and using them wherever possible would remove a lot of clutter. html.cc looks like a C file that was subsequently converted to C++ because it makes little use of C++. If DilloHtmlForm and DilloHtmlInput were given constructors and destructors and managed with new and delete then lots of allocation/deallocation code would be neatly factored out instead of sitting inline with all the other logic. The type element of DilloHtmlInput leads to much horrible case code. If DilloHtmlInput were an abstract class and the different Input types were subclasses then all the type-dependent code could go into virtual methods. This would break up the huge case statements that make much of this code hard to follow. I'll propose patches if people are positive about these suggestions. Jeremy Henty

May 25, 2008

Q: why does Nav_stack_clean() free the data and Nav_stack_truncate() not?

by onepoint＠starurchin.org

On Sun, May 25, 2008 at 09:31:30AM -0400, Jorge Arellano Cid wrote: > Hi Jeremy, > > Nav_stack_truncate() and Nav_stack_insert() are ad-hoc > bindings. They're called just in one place of the code. They're > not clean well-defined functions of an ADT. > > This is "your" mistake. :-) I'll cheerfully abandon my expectations of consistency. :-) But what about memory[1]? Doesn't it leak when Nav_stack_truncate() shrinks bw->nav_stack ? It seems to me that Nav_stack_truncate() should free all those pointers (and therefore Nav_stack_insert() shouldn't free anything). I'll supply a patch if you agree. > The nav.c module is asking for a clean ADT, and if you can work on > it, it's certainly welcomed. Then I hope people will review the patch I sent to the discussion with corvid about the Redirect/Reload crash bug, which is all about the handling of the navigation stack. > BTW, if it's to be rewritten it'd be good to have the TABs > interface in mind. Oh noes, you have spotted my hidden agenda! :-) I was indeed thinking that if I got my head around this it would prepare me for looking at tabs. > This is, if we end with a clean ADT or object, we could have an > array of them, one for each tab, and switch freely among navigation > stacks. As a first step, maybe we could modify uicmd.cc to indirect though a new BrowserWindowSet object. The callbacks would be changed to look up the current BrowserWindow of the BrowserWindowSet . Initially the BrowserWindowSet could be a stub that managed a single BrowserWindow , later it could wrap an array of BrowserWindows . Regards, Jeremy Henty [1] Dillo's memory, that is, not mine. I'm afraid fixing Dillo's memory leaks is easier than fixing my own!

May 25, 2008