edbrowse-dev - development list for edbrowse
 help / color / mirror / Atom feed
From: Adam Thompson <arthompson1990@gmail.com>
To: Karl Dahlke <eklhad@comcast.net>
Cc: Edbrowse-dev@lists.the-brannons.com
Subject: Re: [Edbrowse-dev] [PATCH] Use the list class from the C++ STL,...
Date: Fri, 31 Jan 2014 17:33:14 +0000	[thread overview]
Message-ID: <20140131173314.GC10437@toaster.adamthompson.me.uk> (raw)
In-Reply-To: <20140031080220.eklhad@comcast.net>

On Fri, Jan 31, 2014 at 08:02:20AM -0500, Karl Dahlke wrote:
> > The only useful reason I could see for this change is handling nulls in names
> 
> To me the reason to make a change at all, and I grant you it ain't broke,
> is to get rid of all those warning messages.
> In the linux kernel they encourage you to make such changes,
> even in working code,
> and it's surprising how you can compile the kernel, zillions of
> lines of code, and how few warnings there are.
> In contrast js 24 gives pages and pages of warnings,
> so that you can't tell if any of them should be taken seriously.

Agreed, but the change from char * to string wouldn't fix any of the js24 warnings.
Most of them go away with the -Wno-invalid-offsetof in JS_CXXFLAGS (I know this
isn't nice, but they're Mozilla's warnings which we can't fix).

> In that sense Chris' original patch does the trick, and I'm ok with that.

Yeah, switching to the c++ list class makes sense here.

Also, in the spirit of removing warnings,
my gcc gives me a bunch of warnings in url.c about using int and size_t
interchangably. Do you mind if I go ahead and fix these?

> I just thought it might not be much more work
> to put the struct right in the list, rather than struct * with manual allocs.
> And maybe clean up the code a bit.
> Guess that's Chris' call.

Yeah, but then we've still got the cw->tags array to worry about.
I wonder if we could have the stack as a void * (as you suggest)
and then get whatever needs to use it to use c++?
Which file is it actually rendered in?
If it's one of the existing c++ ones then this'd allow us to get rid of the tag
array, and thus the need for all those pointers.
If not, then I don't know.

> As for line count, I may be able to push from 10 million to 100 million,
> but probably not more than that because of the way
> files are represented with arrays inside,
> having 4 byte indexes etc.
> 
> There is another factor wherein you can run out of your 10 million lines
> not because they are really being used,
> but because there is no garbage collector for lines.
> Make a substitution, and of course you have the new line, but I
> have to keep the old one in case of an undo.
> Make another change and I suppose I could get rid of that original line,
> and yes it is freed (no memory leak),
> but the index inside is not reclaimed.
> The indexes march on towards 10 million.
> So after a long edbrowse session you could run into this, even though you don't
> have 10 million lines active now.
> I've been meaning to address this in some fashion for,
> or er um, at least a decade.
> Well probably push the limit to 100 million first, that's more important
> and easier, then think about better gc procedures for the line indexes.

Yeah. Not sure how to do this without getting the equivalent of memory
fragmentation in the lines array! The only way around this would be to switch
from an array of lines to a linked list of them so you could swap out lines as needed.
This'd be less memory-efficient for small numbers of lines but would make
switching lines in and out much easier.
This'd also get rid of the hard limit on lines completely I think (unless I'm
missing something).
> 
> I have played with jsrt for a while but haven't been able to produce
> your seg fault.
> And the one that I had earlier, reproducible,
> went away with js 24.
> I'll keep fishing around to see if I can trip another one.

Thanks.

Cheers,
Adam.

  reply	other threads:[~2014-01-31 17:33 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-31 13:02 Karl Dahlke
2014-01-31 17:33 ` Adam Thompson [this message]
  -- strict thread matches above, loose matches on Subject: below --
2014-01-31 18:37 Karl Dahlke
2014-01-31 18:48 ` Chris Brannon
2014-02-02 13:43 ` Adam Thompson
2014-01-30 22:25 Karl Dahlke
2014-01-30 21:58 Karl Dahlke
2014-01-30 22:18 ` Chris Brannon
2014-01-31 12:09 ` Adam Thompson
2014-01-31 12:35   ` Chris Brannon
2014-01-30 17:49 Karl Dahlke
2014-01-30 21:23 ` Chris Brannon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140131173314.GC10437@toaster.adamthompson.me.uk \
    --to=arthompson1990@gmail.com \
    --cc=Edbrowse-dev@lists.the-brannons.com \
    --cc=eklhad@comcast.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).