Re: [Emacs-diffs] emacs-25 ee04aed: Fix handling of buffer relocation in regex.c functions

Re: [Emacs-diffs] emacs-25 ee04aed: Fix handling of buffer relocation in regex.c functions

[Stefan Monnier]

> +	  /* Update 'base' due to possible relocation inside re_search_2.  */
> +	  base = current_buffer->text->beg;

I recommend wrapping this in "#if REL_ALLOC", so as to make it clear why
relation might be possible (e.g. to distinguish from GC-triggered
relocation, for example) and to make it clear what could be gained by
getting rid of REL_ALLOC.


        Stefan

Re: [Emacs-diffs] emacs-25 ee04aed: Fix handling of buffer relocation in regex.c functions

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Eli Zaretskii <eliz@gnu.org>
> Date: Sun, 23 Oct 2016 16:31:24 -0400
> 
> > +	  /* Update 'base' due to possible relocation inside re_search_2.  */
> > +	  base = current_buffer->text->beg;
> 
> I recommend wrapping this in "#if REL_ALLOC", so as to make it clear why
> relation might be possible (e.g. to distinguish from GC-triggered
> relocation, for example) and to make it clear what could be gained by
> getting rid of REL_ALLOC.

IMO it makes no sense to condition only this on REL_ALLOC, without
conditioning all the rest.  The bulk is in regex.c, btw.

I think we should consider these nits when we finish fixing the bugs
due to relocation, one way or another.  Until then we don't really
know what parts will stay in the code.

Thanks.

Re: [Emacs-diffs] emacs-25 ee04aed: Fix handling of buffer relocation in regex.c functions

[Stefan Monnier]

> I think we should consider these nits when we finish fixing the bugs
> due to relocation,

I think we should not fix those.  The constraints imposed by
a relocating malloc are too difficult to abide to IMO, so we should just
say no.


        Stefan

Re: [Emacs-diffs] emacs-25 ee04aed: Fix handling of buffer relocation in regex.c functions

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Mon, 24 Oct 2016 09:56:59 -0400
> 
> > I think we should consider these nits when we finish fixing the bugs
> > due to relocation,
> 
> I think we should not fix those.  The constraints imposed by
> a relocating malloc are too difficult to abide to IMO, so we should just
> say no.

It depends on how hard that is.  The last one I fixed was very easy.

The only one left I'm aware of is in boyer_moore, which shouldn't be
hard to fix, either.

More generally, as long as even a single platform we care about uses
ralloc.c, I don't think we can leave those problems unfixed, because
their effects are so nasty.

Re: [Emacs-diffs] emacs-25 ee04aed: Fix handling of buffer relocation in regex.c functions

[Stefan Monnier]

>> > I think we should consider these nits when we finish fixing the bugs
>> > due to relocation,
>> I think we should not fix those.  The constraints imposed by
>> a relocating malloc are too difficult to abide to IMO, so we should just
>> say no.
> It depends on how hard that is.  The last one I fixed was very easy.

How many of us know the constraints?  How many of those know them enough to
think about them when making changes?  How many of those get the answer
right when thinking about those problems?

It's a landmine, just like the gcpros were.

> More generally, as long as even a single platform we care about uses
> ralloc.c, I don't think we can leave those problems unfixed, because
> their effects are so nasty.

That's why I'm advocating getting rid of it.


        Stefan

Re: [Emacs-diffs] emacs-25 ee04aed: Fix handling of buffer relocation in regex.c functions

[Eli Zaretskii]

> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Cc: emacs-devel@gnu.org
> Date: Mon, 24 Oct 2016 15:01:06 -0400
> 
> >> > I think we should consider these nits when we finish fixing the bugs
> >> > due to relocation,
> >> I think we should not fix those.  The constraints imposed by
> >> a relocating malloc are too difficult to abide to IMO, so we should just
> >> say no.
> > It depends on how hard that is.  The last one I fixed was very easy.
> 
> How many of us know the constraints?  How many of those know them enough to
> think about them when making changes?  How many of those get the answer
> right when thinking about those problems?

The same is true for many other parts of Emacs: bidi.c, syntax.c,
composite.c, to name just a few.

> It's a landmine, just like the gcpros were.

Sure.  But we're already in the minefield, might as well be careful
enough to not step on any mine, until we can get out.

> > More generally, as long as even a single platform we care about uses
> > ralloc.c, I don't think we can leave those problems unfixed, because
> > their effects are so nasty.
> 
> That's why I'm advocating getting rid of it.

Same here.  Read my messages.

Re: [Emacs-diffs] emacs-25 ee04aed: Fix handling of buffer relocation in regex.c functions

[Stefan Monnier]

>> How many of us know the constraints?  How many of those know them enough to
>> think about them when making changes?  How many of those get the answer
>> right when thinking about those problems?
> The same is true for many other parts of Emacs: bidi.c, syntax.c,
> composite.c, to name just a few.

I think it's worse because the answer to the question depends on
potentially the whole code (e.g. changing a piece of code such that it
now uses malloc means you have to check every caller (and caller's
caller, ...), and in other direction, when manipulating buffer text you
need to check every function you call (and the functions they call, ...)
to see if they may call malloc).

> Sure.  But we're already in the minefield, might as well be careful
> enough to not step on any mine, until we can get out.

I think we can get out right away.


        Stefan

Re: [Emacs-diffs] emacs-25 ee04aed: Fix handling of buffer relocation in regex.c functions

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Tue, 25 Oct 2016 08:20:25 -0400
> 
> > The same is true for many other parts of Emacs: bidi.c, syntax.c,
> > composite.c, to name just a few.
> 
> I think it's worse because the answer to the question depends on
> potentially the whole code (e.g. changing a piece of code such that it
> now uses malloc means you have to check every caller (and caller's
> caller, ...), and in other direction, when manipulating buffer text you
> need to check every function you call (and the functions they call, ...)
> to see if they may call malloc).

True.  But OTOH, the number of places where we call BYTE_POS_ADDR and
then manipulate the result as a 'char *' is quite small.

> > Sure.  But we're already in the minefield, might as well be careful
> > enough to not step on any mine, until we can get out.
> 
> I think we can get out right away.

No, not yet.  First, we need to make sure that removing ralloc.c from
the build works well and doesn't cause memory fragmentation or other
unwanted side effects, and we need to do that both on GNU/Linux and on
other supported platforms we care about, like *BSD.  If some of these
experience problems, we need to solve them, preferably without using
ralloc.c.  Only when there are no more platforms that use ralloc.c can
we remove all those workarounds from the code.

I agree that there's a good chance we'll achieve this goal soon
enough, but we are not there yet.  We just made the first step today
in that direction.  I'm not even sure we will be able to achieve this
goal before Emacs 25.2 is released.

Re: [Emacs-diffs] emacs-25 ee04aed: Fix handling of buffer relocation in regex.c functions

[Stefan Monnier]

>> I think we can get out right away.
[...]
> I agree that there's a good chance we'll achieve this goal soon
> enough, but we are not there yet.  We just made the first step today
> in that direction.  I'm not even sure we will be able to achieve this
> goal before Emacs 25.2 is released.

Indeed, I meant "right away for master", not "right away for emacs-25".


        Stefan

Re: [Emacs-diffs] emacs-25 ee04aed: Fix handling of buffer relocation in regex.c functions

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Tue, 25 Oct 2016 15:32:47 -0400
> 
> >> I think we can get out right away.
> [...]
> > I agree that there's a good chance we'll achieve this goal soon
> > enough, but we are not there yet.  We just made the first step today
> > in that direction.  I'm not even sure we will be able to achieve this
> > goal before Emacs 25.2 is released.
> 
> Indeed, I meant "right away for master", not "right away for emacs-25".

I don't see the difference.  If it turns out that some platform's
native malloc or gmalloc causes memory fragmentation, we will have to
find a solution, perhaps mmap or something else.  And at least
theoretically some platform could remain with ralloc.c for the
observable future.  We don't want to break people who track master.

Re: [Emacs-diffs] emacs-25 ee04aed: Fix handling of buffer relocation in regex.c functions

[Stefan Monnier]

> I don't see the difference.  If it turns out that some platform's
> native malloc or gmalloc causes memory fragmentation, we will have to
> find a solution, perhaps mmap or something else.

I consider mmap to be a good answer.


        Stefan

Re: [Emacs-diffs] emacs-25 ee04aed: Fix handling of buffer relocation in regex.c functions

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Date: Tue, 25 Oct 2016 15:59:41 -0400
> 
> > I don't see the difference.  If it turns out that some platform's
> > native malloc or gmalloc causes memory fragmentation, we will have to
> > find a solution, perhaps mmap or something else.
> 
> I consider mmap to be a good answer.

If it's needed.  But we don't know yet if it is.

Re: [Emacs-diffs] emacs-25 ee04aed: Fix handling of buffer relocation in regex.c functions

[Stefan Monnier]

> IMO it makes no sense to condition only this on REL_ALLOC, without
> conditioning all the rest.

I didn't mean to condition only this.  Yes, we should condition all code
that is specific to REL_ALLOC with "#if REL_ALLOC".


        Stefan

Re: [Emacs-diffs] emacs-25 ee04aed: Fix handling of buffer relocation in regex.c functions

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Mon, 24 Oct 2016 10:18:59 -0400
> 
> > IMO it makes no sense to condition only this on REL_ALLOC, without
> > conditioning all the rest.
> 
> I didn't mean to condition only this.  Yes, we should condition all code
> that is specific to REL_ALLOC with "#if REL_ALLOC".

I agree, and will do when the dust settles on this.

Re: [Emacs-diffs] emacs-25 ee04aed: Fix handling of buffer relocation in regex.c functions

[Eli Zaretskii]

> Date: Mon, 24 Oct 2016 18:35:39 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> > From: Stefan Monnier <monnier@iro.umontreal.ca>
> > Cc: emacs-devel@gnu.org
> > Date: Mon, 24 Oct 2016 10:18:59 -0400
> > 
> > > IMO it makes no sense to condition only this on REL_ALLOC, without
> > > conditioning all the rest.
> > 
> > I didn't mean to condition only this.  Yes, we should condition all code
> > that is specific to REL_ALLOC with "#if REL_ALLOC".
> 
> I agree, and will do when the dust settles on this.

Done.