unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Enlarge MAX_ALLOCA?
@ 2014-06-19 16:02 Eli Zaretskii
  2014-06-19 16:23 ` David Kastrup
  2014-06-19 18:28 ` Stefan Monnier
  0 siblings, 2 replies; 33+ messages in thread
From: Eli Zaretskii @ 2014-06-19 16:02 UTC (permalink / raw)
  To: emacs-devel

Does anyone see problems with the change below, which raises the bar
for 'alloca' to 64KB?  Are there any systems out there that we care
about whose stack is so small as to make this dangerous?

Why 64KB?  Because that's the size of the work area coding.c allocates
whenever it needs to encode or decode something.  It turns out we do
this a lot, e.g., every redisplay calls file-readable-p on the icon
image files, which needs to encode the file name.  While the work area
is immediately free'd, I think allocating such a large buffer so much
has a potential of creating an unnecessary memory pressure on 'malloc',
and perhaps cause excess fragmentation and/or enlarge memory footprint
in some cases.

Thoughts?

=== modified file 'src/lisp.h'
--- src/lisp.h	2014-06-17 16:09:19 +0000
+++ src/lisp.h	2014-06-19 15:37:17 +0000
@@ -4425,7 +4425,7 @@ extern void init_system_name (void);
 /* SAFE_ALLOCA normally allocates memory on the stack, but if size is
    larger than MAX_ALLOCA, use xmalloc to avoid overflowing the stack.  */
 
-enum MAX_ALLOCA { MAX_ALLOCA = 16 * 1024 };
+enum MAX_ALLOCA { MAX_ALLOCA = 64 * 1024 };
 
 extern void *record_xmalloc (size_t) ATTRIBUTE_ALLOC_SIZE ((1));
 
@@ -4434,7 +4434,7 @@ extern void *record_xmalloc (size_t) ATT
 
 /* SAFE_ALLOCA allocates a simple buffer.  */
 
-#define SAFE_ALLOCA(size) ((size) < MAX_ALLOCA	\
+#define SAFE_ALLOCA(size) ((size) <= MAX_ALLOCA	\
 			   ? alloca (size)	\
 			   : (sa_must_free = true, record_xmalloc (size)))
 
@@ -4469,7 +4469,7 @@ extern void *record_xmalloc (size_t) ATT
 
 #define SAFE_ALLOCA_LISP(buf, nelt)			       \
   do {							       \
-    if ((nelt) < MAX_ALLOCA / word_size)		       \
+    if ((nelt) <= MAX_ALLOCA / word_size)		       \
       (buf) = alloca ((nelt) * word_size);		       \
     else if ((nelt) < min (PTRDIFF_MAX, SIZE_MAX) / word_size) \
       {							       \




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-19 16:02 Enlarge MAX_ALLOCA? Eli Zaretskii
@ 2014-06-19 16:23 ` David Kastrup
  2014-06-19 16:48   ` Eli Zaretskii
  2014-06-19 18:21   ` Stefan Monnier
  2014-06-19 18:28 ` Stefan Monnier
  1 sibling, 2 replies; 33+ messages in thread
From: David Kastrup @ 2014-06-19 16:23 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

> -#define SAFE_ALLOCA(size) ((size) < MAX_ALLOCA	\
> +#define SAFE_ALLOCA(size) ((size) <= MAX_ALLOCA	\
>  			   ? alloca (size)	\
>  			   : (sa_must_free = true, record_xmalloc (size)))
>  
> @@ -4469,7 +4469,7 @@ extern void *record_xmalloc (size_t) ATT
>  
>  #define SAFE_ALLOCA_LISP(buf, nelt)			       \
>    do {							       \
> -    if ((nelt) < MAX_ALLOCA / word_size)		       \
> +    if ((nelt) <= MAX_ALLOCA / word_size)		       \
>        (buf) = alloca ((nelt) * word_size);		       \
>      else if ((nelt) < min (PTRDIFF_MAX, SIZE_MAX) / word_size) \
>        {							       \

Bad idea to change < to <= here.  If there is a hard limit due to short
offsets or similar (and if there weren't, why bother at all?), then
allocating a full 64kB might be a bad idea.

With regard to short addressing modes, 32kB might be more appropriate,
and if the normal stack frame is included in that address range, 30kB
might be more cautious.

64kB feels arbitrary.  I cannot really think of an architecture where
64kB would be feasible and 128kB not.  ±32kB is a plausible offset for
some architectures.

-- 
David Kastrup




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-19 16:23 ` David Kastrup
@ 2014-06-19 16:48   ` Eli Zaretskii
  2014-06-19 17:04     ` David Kastrup
  2014-06-19 18:21   ` Stefan Monnier
  1 sibling, 1 reply; 33+ messages in thread
From: Eli Zaretskii @ 2014-06-19 16:48 UTC (permalink / raw)
  To: David Kastrup; +Cc: emacs-devel

> From: David Kastrup <dak@gnu.org>
> Date: Thu, 19 Jun 2014 18:23:26 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > -#define SAFE_ALLOCA(size) ((size) < MAX_ALLOCA	\
> > +#define SAFE_ALLOCA(size) ((size) <= MAX_ALLOCA	\
> >  			   ? alloca (size)	\
> >  			   : (sa_must_free = true, record_xmalloc (size)))
> >  
> > @@ -4469,7 +4469,7 @@ extern void *record_xmalloc (size_t) ATT
> >  
> >  #define SAFE_ALLOCA_LISP(buf, nelt)			       \
> >    do {							       \
> > -    if ((nelt) < MAX_ALLOCA / word_size)		       \
> > +    if ((nelt) <= MAX_ALLOCA / word_size)		       \
> >        (buf) = alloca ((nelt) * word_size);		       \
> >      else if ((nelt) < min (PTRDIFF_MAX, SIZE_MAX) / word_size) \
> >        {							       \
> 
> Bad idea to change < to <= here.

The original macros were inconsistent: some used < and some <=, so I
changed them.

> If there is a hard limit due to short offsets or similar (and if
> there weren't, why bother at all?), then allocating a full 64kB
> might be a bad idea.

Is there really such a system?  If so, which one?  And why would that
be a worse idea than to allocate the same 64KB off the heap (which is
what that macro does in the 'else' clause?  What am I missing?

> 64kB feels arbitrary.

I explained my rationale for choosing this value.



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-19 16:48   ` Eli Zaretskii
@ 2014-06-19 17:04     ` David Kastrup
  2014-06-19 17:14       ` Eli Zaretskii
  0 siblings, 1 reply; 33+ messages in thread
From: David Kastrup @ 2014-06-19 17:04 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: David Kastrup <dak@gnu.org>
>> Date: Thu, 19 Jun 2014 18:23:26 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > -#define SAFE_ALLOCA(size) ((size) < MAX_ALLOCA	\
>> > +#define SAFE_ALLOCA(size) ((size) <= MAX_ALLOCA	\
>> >  			   ? alloca (size)	\
>> >  			   : (sa_must_free = true, record_xmalloc (size)))
>> >  
>> > @@ -4469,7 +4469,7 @@ extern void *record_xmalloc (size_t) ATT
>> >  
>> >  #define SAFE_ALLOCA_LISP(buf, nelt)			       \
>> >    do {							       \
>> > -    if ((nelt) < MAX_ALLOCA / word_size)		       \
>> > +    if ((nelt) <= MAX_ALLOCA / word_size)		       \
>> >        (buf) = alloca ((nelt) * word_size);		       \
>> >      else if ((nelt) < min (PTRDIFF_MAX, SIZE_MAX) / word_size) \
>> >        {							       \
>> 
>> Bad idea to change < to <= here.
>
> The original macros were inconsistent: some used < and some <=, so I
> changed them.
>
>> If there is a hard limit due to short offsets or similar (and if
>> there weren't, why bother at all?), then allocating a full 64kB
>> might be a bad idea.
>
> Is there really such a system?  If so, which one?

Either your limit has a rationale in machine architectures or not.  If
it has: the C standard guarantees that you are allowed to take the
address _after_ an array.

The 68k architecture has short offsets (-32768..+32767) for addressing
off an address register such as the stack pointer.

> And why would that be a worse idea than to allocate the same 64KB off
> the heap (which is what that macro does in the 'else' clause?  What am
> I missing?

Heap addressing will not employ short offsets/pointers on such
architectures.

>> 64kB feels arbitrary.
>
> I explained my rationale for choosing this value.

The explanation was:

> Why 64KB?  Because that's the size of the work area coding.c allocates
> whenever it needs to encode or decode something.  It turns out we do
> this a lot, e.g., every redisplay calls file-readable-p on the icon
> image files, which needs to encode the file name.  While the work area
> is immediately free'd, I think allocating such a large buffer so much
> has a potential of creating an unnecessary memory pressure on
> 'malloc', and perhaps cause excess fragmentation and/or enlarge memory
> footprint in some cases.

That's not related to an architecture restraint.  In fact, it merely
follows the arbitrary definition

#define CHARBUF_SIZE 0x4000

Arbitrary because this is not a lookup table size but a buffer size for
portioned conversion.  Instead of doubling MAX_ALLOCA, it would seem to
make more sense to reduce CHARBUF_SIZE to something making it fit better
on the stack if this is performance relevant.

As I said: there are architectural reasons (short addressing mode)
making somewhat less than 32kB a good choice on some architectures.

-- 
David Kastrup



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-19 17:04     ` David Kastrup
@ 2014-06-19 17:14       ` Eli Zaretskii
  2014-06-19 17:36         ` David Kastrup
  0 siblings, 1 reply; 33+ messages in thread
From: Eli Zaretskii @ 2014-06-19 17:14 UTC (permalink / raw)
  To: David Kastrup; +Cc: emacs-devel

> From: David Kastrup <dak@gnu.org>
> Cc: emacs-devel@gnu.org
> Date: Thu, 19 Jun 2014 19:04:09 +0200
> 
> >> If there is a hard limit due to short offsets or similar (and if
> >> there weren't, why bother at all?), then allocating a full 64kB
> >> might be a bad idea.
> >
> > Is there really such a system?  If so, which one?
> 
> Either your limit has a rationale in machine architectures or not.  If
> it has: the C standard guarantees that you are allowed to take the
> address _after_ an array.
> 
> The 68k architecture has short offsets (-32768..+32767) for addressing
> off an address register such as the stack pointer.

But we don't support 68k anymore, AFAIK.

> >> 64kB feels arbitrary.
> >
> > I explained my rationale for choosing this value.
> 
> The explanation was:
> 
> > Why 64KB?  Because that's the size of the work area coding.c allocates
> > whenever it needs to encode or decode something.  It turns out we do
> > this a lot, e.g., every redisplay calls file-readable-p on the icon
> > image files, which needs to encode the file name.  While the work area
> > is immediately free'd, I think allocating such a large buffer so much
> > has a potential of creating an unnecessary memory pressure on
> > 'malloc', and perhaps cause excess fragmentation and/or enlarge memory
> > footprint in some cases.
> 
> That's not related to an architecture restraint.  In fact, it merely
> follows the arbitrary definition
> 
> #define CHARBUF_SIZE 0x4000

Indeed.  As I stated.

> Arbitrary because this is not a lookup table size but a buffer size for
> portioned conversion.  Instead of doubling MAX_ALLOCA, it would seem to
> make more sense to reduce CHARBUF_SIZE to something making it fit better
> on the stack if this is performance relevant.

I don't think CHARBUF_SIZE is too arbitrary, but I will let the
experts explain why they chose it.

> As I said: there are architectural reasons (short addressing mode)
> making somewhat less than 32kB a good choice on some architectures.

But do we support such architectures?  I think we don't.



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-19 17:14       ` Eli Zaretskii
@ 2014-06-19 17:36         ` David Kastrup
  2014-06-19 17:51           ` Eli Zaretskii
  0 siblings, 1 reply; 33+ messages in thread
From: David Kastrup @ 2014-06-19 17:36 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> As I said: there are architectural reasons (short addressing mode)
>> making somewhat less than 32kB a good choice on some architectures.
>
> But do we support such architectures?  I think we don't.

(info "(gcc) Machine constraints") lists a lot of architectures having
constraints (and consequently instructions) with 16-bit offsets.

-- 
David Kastrup



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-19 17:36         ` David Kastrup
@ 2014-06-19 17:51           ` Eli Zaretskii
  0 siblings, 0 replies; 33+ messages in thread
From: Eli Zaretskii @ 2014-06-19 17:51 UTC (permalink / raw)
  To: David Kastrup; +Cc: emacs-devel

> From: David Kastrup <dak@gnu.org>
> Cc: emacs-devel@gnu.org
> Date: Thu, 19 Jun 2014 19:36:55 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> As I said: there are architectural reasons (short addressing mode)
> >> making somewhat less than 32kB a good choice on some architectures.
> >
> > But do we support such architectures?  I think we don't.
> 
> (info "(gcc) Machine constraints") lists a lot of architectures having
> constraints (and consequently instructions) with 16-bit offsets.

The mere existence of such instructions doesn't mean they contradict
large buffers allocated off the stack.



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-19 16:23 ` David Kastrup
  2014-06-19 16:48   ` Eli Zaretskii
@ 2014-06-19 18:21   ` Stefan Monnier
  2014-06-19 21:13     ` David Kastrup
  2014-06-20  8:38     ` Dmitry Antipov
  1 sibling, 2 replies; 33+ messages in thread
From: Stefan Monnier @ 2014-06-19 18:21 UTC (permalink / raw)
  To: David Kastrup; +Cc: emacs-devel

> 64kB feels arbitrary.  I cannot really think of an architecture where
> 64kB would be feasible and 128kB not.  ±32kB is a plausible offset for
> some architectures.

This has nothing to do with machine architectures.
It's only related to the OS chosen size of the stack.


        Stefan



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-19 16:02 Enlarge MAX_ALLOCA? Eli Zaretskii
  2014-06-19 16:23 ` David Kastrup
@ 2014-06-19 18:28 ` Stefan Monnier
  2014-06-19 18:38   ` Eli Zaretskii
  1 sibling, 1 reply; 33+ messages in thread
From: Stefan Monnier @ 2014-06-19 18:28 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

> Does anyone see problems with the change below, which raises the bar
> for 'alloca' to 64KB?  Are there any systems out there that we care
> about whose stack is so small as to make this dangerous?

It makes the potential max increment of stack space "per
lisp-eval-depth" larger, and hence increases the risk that we'll eat up
our stack before we bump into max-lisp-eval-depth.

The idea behind the 16KB limit is that the cost of malloc+free is likely
to be negligible compared the filling and using 16KB of data.

> Why 64KB?  Because that's the size of the work area coding.c allocates
> whenever it needs to encode or decode something.  It turns out we do
> this a lot, e.g., every redisplay calls file-readable-p on the icon
> image files, which needs to encode the file name.  While the work area
> is immediately free'd, I think allocating such a large buffer so much
> has a potential of creating an unnecessary memory pressure on 'malloc',
> and perhaps cause excess fragmentation and/or enlarge memory footprint
> in some cases.

I think it makes a lot of sense to try and allocate this space on the
stack when decoding file names, but why does it allocate such a huge
buffer just to en/decode a puny file name?

If the malloc/free done for this encode/decode is relatively costly,
maybe it's because we allocate too much space compared to what we use.


        Stefan



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-19 18:28 ` Stefan Monnier
@ 2014-06-19 18:38   ` Eli Zaretskii
  2014-06-19 20:37     ` Stefan Monnier
  2014-06-21 13:01     ` K. Handa
  0 siblings, 2 replies; 33+ messages in thread
From: Eli Zaretskii @ 2014-06-19 18:38 UTC (permalink / raw)
  To: Stefan Monnier, Kenichi Handa; +Cc: emacs-devel

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: emacs-devel@gnu.org
> Date: Thu, 19 Jun 2014 14:28:08 -0400
> 
> > Does anyone see problems with the change below, which raises the bar
> > for 'alloca' to 64KB?  Are there any systems out there that we care
> > about whose stack is so small as to make this dangerous?
> 
> It makes the potential max increment of stack space "per
> lisp-eval-depth" larger, and hence increases the risk that we'll eat up
> our stack before we bump into max-lisp-eval-depth.

Which functions relevant to max-lisp-eval-depth use SAFE_ALLOCA?

> The idea behind the 16KB limit is that the cost of malloc+free is likely
> to be negligible compared the filling and using 16KB of data.

I'm talking about the long-run costs, not the immediate ones.

> I think it makes a lot of sense to try and allocate this space on the
> stack when decoding file names, but why does it allocate such a huge
> buffer just to en/decode a puny file name?

That buffer is fixed in size, I don't know what.  Perhaps it's hard to
know in advance how much we will need.  I hope Handa-san could explain.

> If the malloc/free done for this encode/decode is relatively costly,
> maybe it's because we allocate too much space compared to what we use.

Again, the problem is memory fragmentation and the resulting large
footprint, not the cost of the allocation.



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-19 18:38   ` Eli Zaretskii
@ 2014-06-19 20:37     ` Stefan Monnier
  2014-06-20  7:08       ` Eli Zaretskii
  2014-06-21 13:01     ` K. Handa
  1 sibling, 1 reply; 33+ messages in thread
From: Stefan Monnier @ 2014-06-19 20:37 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Kenichi Handa, emacs-devel

>> It makes the potential max increment of stack space "per
>> lisp-eval-depth" larger, and hence increases the risk that we'll eat up
>> our stack before we bump into max-lisp-eval-depth.
> Which functions relevant to max-lisp-eval-depth use SAFE_ALLOCA?

I don't know.  Maybe none, but that would surprise me.

>> I think it makes a lot of sense to try and allocate this space on the
>> stack when decoding file names, but why does it allocate such a huge
>> buffer just to en/decode a puny file name?
> That buffer is fixed in size, I don't know why.  Perhaps it's hard to
> know in advance how much we will need.

Supposedly, every coding system comes with a "blow up factor" (can't
remember the name we use) which should be usable to compute a safe
upper-bound.

> I hope Handa-san could explain.

Handa?

> Again, the problem is memory fragmentation and the resulting large
> footprint, not the cost of the allocation.

Is this memory fragmentation problem hypothetical, or have we seen
evidence of it?


        Stefan



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-19 18:21   ` Stefan Monnier
@ 2014-06-19 21:13     ` David Kastrup
  2014-06-20  7:10       ` Eli Zaretskii
  2014-06-20  8:38     ` Dmitry Antipov
  1 sibling, 1 reply; 33+ messages in thread
From: David Kastrup @ 2014-06-19 21:13 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> 64kB feels arbitrary.  I cannot really think of an architecture where
>> 64kB would be feasible and 128kB not.  ±32kB is a plausible offset for
>> some architectures.
>
> This has nothing to do with machine architectures.
> It's only related to the OS chosen size of the stack.

It would be "only related to stack size" when there was a strictly
limited number of alloca allocations in the whole call stack since the
limit is per alloca, not per Emacs invocation.

-- 
David Kastrup



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-19 20:37     ` Stefan Monnier
@ 2014-06-20  7:08       ` Eli Zaretskii
  2014-06-20 13:02         ` Stefan Monnier
  0 siblings, 1 reply; 33+ messages in thread
From: Eli Zaretskii @ 2014-06-20  7:08 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: handa, emacs-devel

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Kenichi Handa <handa@gnu.org>,  emacs-devel@gnu.org
> Date: Thu, 19 Jun 2014 16:37:07 -0400
> 
> >> It makes the potential max increment of stack space "per
> >> lisp-eval-depth" larger, and hence increases the risk that we'll eat up
> >> our stack before we bump into max-lisp-eval-depth.
> > Which functions relevant to max-lisp-eval-depth use SAFE_ALLOCA?
> 
> I don't know.  Maybe none, but that would surprise me.

I will take a look.

Alternatively, how about using a different limit just in the 2 users
of ALLOC_CONVERSION_WORK_AREA?

> >> I think it makes a lot of sense to try and allocate this space on the
> >> stack when decoding file names, but why does it allocate such a huge
> >> buffer just to en/decode a puny file name?
> > That buffer is fixed in size, I don't know why.  Perhaps it's hard to
> > know in advance how much we will need.
> 
> Supposedly, every coding system comes with a "blow up factor" (can't
> remember the name we use) which should be usable to compute a safe
> upper-bound.

I don't think the work area is what you think it is.  It is not for
the result of the encoding/decoding; that is allocated elsewhere, and
indeed uses a dynamic size computed from the source size.  This work
area is for something else: for handling composition annotations
(whatever that is).  See handle_composition_annotation and
produce_annotation in coding.c.

> > Again, the problem is memory fragmentation and the resulting large
> > footprint, not the cost of the allocation.
> 
> Is this memory fragmentation problem hypothetical, or have we seen
> evidence of it?

Hypothetical.  I just don't like seeing such frequent sequences of
malloc(64K)/free() one after the other several times a second in a
live session.



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-19 21:13     ` David Kastrup
@ 2014-06-20  7:10       ` Eli Zaretskii
  2014-06-20  8:08         ` David Kastrup
  0 siblings, 1 reply; 33+ messages in thread
From: Eli Zaretskii @ 2014-06-20  7:10 UTC (permalink / raw)
  To: David Kastrup; +Cc: monnier, emacs-devel

> From: David Kastrup <dak@gnu.org>
> Date: Thu, 19 Jun 2014 23:13:47 +0200
> Cc: emacs-devel@gnu.org
> 
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> >> 64kB feels arbitrary.  I cannot really think of an architecture where
> >> 64kB would be feasible and 128kB not.  ±32kB is a plausible offset for
> >> some architectures.
> >
> > This has nothing to do with machine architectures.
> > It's only related to the OS chosen size of the stack.
> 
> It would be "only related to stack size" when there was a strictly
> limited number of alloca allocations in the whole call stack since the
> limit is per alloca, not per Emacs invocation.

I'm not sure I follow your reasoning.  Can you elaborate, please?




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-20  7:10       ` Eli Zaretskii
@ 2014-06-20  8:08         ` David Kastrup
  0 siblings, 0 replies; 33+ messages in thread
From: David Kastrup @ 2014-06-20  8:08 UTC (permalink / raw)
  To: emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> From: David Kastrup <dak@gnu.org>
>> Date: Thu, 19 Jun 2014 23:13:47 +0200
>> Cc: emacs-devel@gnu.org
>> 
>> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> 
>> >> 64kB feels arbitrary.  I cannot really think of an architecture where
>> >> 64kB would be feasible and 128kB not.  ±32kB is a plausible offset for
>> >> some architectures.
>> >
>> > This has nothing to do with machine architectures.
>> > It's only related to the OS chosen size of the stack.
>> 
>> It would be "only related to stack size" when there was a strictly
>> limited number of alloca allocations in the whole call stack since the
>> limit is per alloca, not per Emacs invocation.
>
> I'm not sure I follow your reasoning.  Can you elaborate, please?

I don't see that anything I say would register, so I don't see the point
in further wasting my time.

-- 
David Kastrup




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-19 18:21   ` Stefan Monnier
  2014-06-19 21:13     ` David Kastrup
@ 2014-06-20  8:38     ` Dmitry Antipov
  2014-06-20  8:56       ` Eli Zaretskii
  2014-06-20  9:26       ` Andreas Schwab
  1 sibling, 2 replies; 33+ messages in thread
From: Dmitry Antipov @ 2014-06-20  8:38 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: David Kastrup, emacs-devel

On 06/19/2014 10:21 PM, Stefan Monnier wrote:

>> 64kB feels arbitrary.  I cannot really think of an architecture where
>> 64kB would be feasible and 128kB not.  ±32kB is a plausible offset for
>> some architectures.
>
> This has nothing to do with machine architectures.
> It's only related to the OS chosen size of the stack.

IIUC this means that if alloca is limited to < 32K, stack may be expanded
with just one instruction because increment fits into an immediate operand.
Otherwise there should be a few more load/store/add instructions.

Dmitry





^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-20  8:38     ` Dmitry Antipov
@ 2014-06-20  8:56       ` Eli Zaretskii
  2014-06-20  9:26       ` Andreas Schwab
  1 sibling, 0 replies; 33+ messages in thread
From: Eli Zaretskii @ 2014-06-20  8:56 UTC (permalink / raw)
  To: Dmitry Antipov; +Cc: dak, monnier, emacs-devel

> Date: Fri, 20 Jun 2014 12:38:03 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> Cc: David Kastrup <dak@gnu.org>, emacs-devel@gnu.org
> 
> On 06/19/2014 10:21 PM, Stefan Monnier wrote:
> 
> >> 64kB feels arbitrary.  I cannot really think of an architecture where
> >> 64kB would be feasible and 128kB not.  ±32kB is a plausible offset for
> >> some architectures.
> >
> > This has nothing to do with machine architectures.
> > It's only related to the OS chosen size of the stack.
> 
> IIUC this means that if alloca is limited to < 32K, stack may be expanded
> with just one instruction because increment fits into an immediate operand.
> Otherwise there should be a few more load/store/add instructions.

OK, but is that worse than using xmalloc in that case?




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-20  8:38     ` Dmitry Antipov
  2014-06-20  8:56       ` Eli Zaretskii
@ 2014-06-20  9:26       ` Andreas Schwab
  2014-06-20  9:38         ` David Kastrup
  1 sibling, 1 reply; 33+ messages in thread
From: Andreas Schwab @ 2014-06-20  9:26 UTC (permalink / raw)
  To: Dmitry Antipov; +Cc: David Kastrup, Stefan Monnier, emacs-devel

Dmitry Antipov <dmantipov@yandex.ru> writes:

> IIUC this means that if alloca is limited to < 32K, stack may be expanded
> with just one instruction because increment fits into an immediate operand.
> Otherwise there should be a few more load/store/add instructions.

If you use alloca you have a variable sized stack frame, so the offset
will be variable as well.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-20  9:26       ` Andreas Schwab
@ 2014-06-20  9:38         ` David Kastrup
  0 siblings, 0 replies; 33+ messages in thread
From: David Kastrup @ 2014-06-20  9:38 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: Dmitry Antipov, Stefan Monnier, emacs-devel

Andreas Schwab <schwab@linux-m68k.org> writes:

> Dmitry Antipov <dmantipov@yandex.ru> writes:
>
>> IIUC this means that if alloca is limited to < 32K, stack may be expanded
>> with just one instruction because increment fits into an immediate operand.
>> Otherwise there should be a few more load/store/add instructions.
>
> If you use alloca you have a variable sized stack frame, so the offset
> will be variable as well.

Ah right, alloca is generally not something affecting the code
generation of unrelated code, so arrays allocated with alloca do not
change the offsets of scalar stack variables which are usually addressed
off a separate frame pointer unaffected by alloca.

So the addressing mode restrictions I brought up would be a red herring,
even when talking about something like 68k architectures.  So it seems
like indeed just the total stack size available (or not available) in
connection with the expected stack depth particularly including
functions using alloca that should govern the chosen sizes.

-- 
David Kastrup



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-20  7:08       ` Eli Zaretskii
@ 2014-06-20 13:02         ` Stefan Monnier
  2014-06-20 13:18           ` Eli Zaretskii
  0 siblings, 1 reply; 33+ messages in thread
From: Stefan Monnier @ 2014-06-20 13:02 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: handa, emacs-devel

> Alternatively, how about using a different limit just in the 2 users
> of ALLOC_CONVERSION_WORK_AREA?

Why not just use `alloca' directly for those cases, then?

> I don't think the work area is what you think it is.  It is not for
> the result of the encoding/decoding; that is allocated elsewhere, and
> indeed uses a dynamic size computed from the source size.  This work
> area is for something else: for handling composition annotations
> (whatever that is).  See handle_composition_annotation and
> produce_annotation in coding.c.

I see, then indeed I have no idea what that's about.

>> > Again, the problem is memory fragmentation and the resulting large
>> > footprint, not the cost of the allocation.
>> Is this memory fragmentation problem hypothetical, or have we seen
>> evidence of it?
> Hypothetical.  I just don't like seeing such frequent sequences of
> malloc(64K)/free() one after the other several times a second in a
> live session.

I'd expect any non-toy implementation of malloc/free to handle this
without any serious risk of fragmentation, to tell you the truth.


        Stefan



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-20 13:02         ` Stefan Monnier
@ 2014-06-20 13:18           ` Eli Zaretskii
  2014-06-20 14:43             ` Stefan Monnier
  0 siblings, 1 reply; 33+ messages in thread
From: Eli Zaretskii @ 2014-06-20 13:18 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: handa, emacs-devel

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: handa@gnu.org,  emacs-devel@gnu.org
> Date: Fri, 20 Jun 2014 09:02:19 -0400
> 
> > Alternatively, how about using a different limit just in the 2 users
> > of ALLOC_CONVERSION_WORK_AREA?
> 
> Why not just use `alloca' directly for those cases, then?

With an assertion in case the size is enlarged some day?

> >> Is this memory fragmentation problem hypothetical, or have we seen
> >> evidence of it?
> > Hypothetical.  I just don't like seeing such frequent sequences of
> > malloc(64K)/free() one after the other several times a second in a
> > live session.
> 
> I'd expect any non-toy implementation of malloc/free to handle this
> without any serious risk of fragmentation, to tell you the truth.

gmalloc doesn't, at least not with ralloc.c.  One of my systems keeps
growing in memory, and the only reason I could find is these constant
allocations.  I don't know whether it's fragmentation or not, but the
fact is, once every few dozens of such calls, sbrk gets called to
enlarge the brk value.  Whenever sbrk is called, I always see
ALLOC_CONVERSION_WORK_AREA on the callstack asking for the same 64KB
allocation.  The higher-level code that triggers that is either
file-readable-p test for a tool-bar icon, or current-time-zone (called
frequently because I customized display-time to update the mode line
every 5 sec).

It could be that this is due to the known problems with ralloc.c,
whereby it sometimes loses track of some bloc it manages, and then all
the blocs below that become "trapped" and cannot be relinquished back
to the system.  I plan to try this with the trunk version soon, where
ralloc is no longer used.

Strange thing is, this only happens on 1 of 3 systems I use regularly,
which all have almost identical setup.



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-20 13:18           ` Eli Zaretskii
@ 2014-06-20 14:43             ` Stefan Monnier
  2014-06-20 14:50               ` Eli Zaretskii
  2014-06-20 15:15               ` Herring, Davis
  0 siblings, 2 replies; 33+ messages in thread
From: Stefan Monnier @ 2014-06-20 14:43 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: handa, emacs-devel

>> > Alternatively, how about using a different limit just in the 2 users
>> > of ALLOC_CONVERSION_WORK_AREA?
>> Why not just use `alloca' directly for those cases, then?
> With an assertion in case the size is enlarged some day?

OK.

>> >> Is this memory fragmentation problem hypothetical, or have we seen
>> >> evidence of it?
>> > Hypothetical.  I just don't like seeing such frequent sequences of
>> > malloc(64K)/free() one after the other several times a second in a
>> > live session.
>> I'd expect any non-toy implementation of malloc/free to handle this
>> without any serious risk of fragmentation, to tell you the truth.
> gmalloc doesn't, at least not with ralloc.c.  One of my systems keeps
> growing in memory, and the only reason I could find is these constant
> allocations.

I see, so it's not just hypothetical (tho the evidence is
circumstantial, as is usually the case: proving fragmentation is
difficult).

So I think the way to go is the following:
- see if we can allocate this area less frequently (e.g. if it's for
  handling compositions, it seems like it shouldn't be needed in your
  cases where you encode file names (or do your file names have parts
  that require compositions?)).  I.e. delay the allocation to when we
  really need it.
- try to reduce the size of this area.
- if the above two is not sufficient, try and just use plain `alloca'
  (tho the "delay allocation" part above might make it
  difficult/impossible to use alloca).

> Strange thing is, this only happens on 1 of 3 systems I use regularly,
> which all have almost identical setup.

Try to put him closer to the other 2 systems, so he'll feel ashamed.


        Stefan



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-20 14:43             ` Stefan Monnier
@ 2014-06-20 14:50               ` Eli Zaretskii
  2014-06-20 15:15               ` Herring, Davis
  1 sibling, 0 replies; 33+ messages in thread
From: Eli Zaretskii @ 2014-06-20 14:50 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: handa, emacs-devel

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: handa@gnu.org,  emacs-devel@gnu.org
> Date: Fri, 20 Jun 2014 10:43:10 -0400
> 
> So I think the way to go is the following:
> - see if we can allocate this area less frequently (e.g. if it's for
>   handling compositions, it seems like it shouldn't be needed in your
>   cases where you encode file names (or do your file names have parts
>   that require compositions?)).  I.e. delay the allocation to when we
>   really need it.
> - try to reduce the size of this area.
> - if the above two is not sufficient, try and just use plain `alloca'
>   (tho the "delay allocation" part above might make it
>   difficult/impossible to use alloca).

I went for the last one first, let's see what happens.

> > Strange thing is, this only happens on 1 of 3 systems I use regularly,
> > which all have almost identical setup.
> 
> Try to put him closer to the other 2 systems, so he'll feel ashamed.

One of them is as close to it as close can be: they are 2 boxes
standing right next to each other.  I saw no signs of shame.



^ permalink raw reply	[flat|nested] 33+ messages in thread

* RE: Enlarge MAX_ALLOCA?
  2014-06-20 14:43             ` Stefan Monnier
  2014-06-20 14:50               ` Eli Zaretskii
@ 2014-06-20 15:15               ` Herring, Davis
  2014-06-20 15:44                 ` Dmitry Antipov
  1 sibling, 1 reply; 33+ messages in thread
From: Herring, Davis @ 2014-06-20 15:15 UTC (permalink / raw)
  To: Stefan Monnier, Eli Zaretskii; +Cc: handa@gnu.org, emacs-devel@gnu.org

> - see if we can allocate this area less frequently (e.g. if it's for
>   handling compositions, it seems like it shouldn't be needed in your
>   cases where you encode file names (or do your file names have parts
>   that require compositions?)).  I.e. delay the allocation to when we
>   really need it.

We could also allocate it less frequently by allocating it statically (unless en/decoding can be reentrant, which seems unlikely).

Davis



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-20 15:15               ` Herring, Davis
@ 2014-06-20 15:44                 ` Dmitry Antipov
  2014-06-20 18:36                   ` Eli Zaretskii
  0 siblings, 1 reply; 33+ messages in thread
From: Dmitry Antipov @ 2014-06-20 15:44 UTC (permalink / raw)
  To: Stefan Monnier, Eli Zaretskii; +Cc: handa@gnu.org, emacs-devel@gnu.org

On 06/20/2014 07:15 PM, Herring, Davis wrote:

> We could also allocate it less frequently by allocating it statically
> (unless en/decoding can be reentrant, which seems unlikely).

We can also estimate its size before blindly allocating 64 (or 32, or
whatever else). For example, if we decode from 16-character string
to *code-conversion-work*, we need...hm, what's the upper bound?
May be 16 * MAX_MULTIBYTE_LENGTH?

Dmitry




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-20 15:44                 ` Dmitry Antipov
@ 2014-06-20 18:36                   ` Eli Zaretskii
  0 siblings, 0 replies; 33+ messages in thread
From: Eli Zaretskii @ 2014-06-20 18:36 UTC (permalink / raw)
  To: Dmitry Antipov; +Cc: handa, monnier, emacs-devel

> Date: Fri, 20 Jun 2014 19:44:46 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> CC: "Herring, Davis" <herring@lanl.gov>, "handa@gnu.org" <handa@gnu.org>, 
>  "emacs-devel@gnu.org" <emacs-devel@gnu.org>
> 
> We can also estimate its size before blindly allocating 64 (or 32, or
> whatever else). For example, if we decode from 16-character string
> to *code-conversion-work*, we need...hm, what's the upper bound?
> May be 16 * MAX_MULTIBYTE_LENGTH?

Once again, this is not what the work area is used for, please read my
message where I mentioned the 2 functions which need this work area.



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-19 18:38   ` Eli Zaretskii
  2014-06-19 20:37     ` Stefan Monnier
@ 2014-06-21 13:01     ` K. Handa
  2014-06-21 13:59       ` Eli Zaretskii
  2014-06-21 15:19       ` David Kastrup
  1 sibling, 2 replies; 33+ messages in thread
From: K. Handa @ 2014-06-21 13:01 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel

In article <83lhsssq1b.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:

> That buffer is fixed in size, I don't know what.  Perhaps it's hard to
> know in advance how much we will need.  I hope Handa-san could explain.

The reason of making it fixed size is that I was just lazy
and didn't expect that decoder/encoder were called so
frequently.  :-( The reaons of 0x4000 was that most of the
elisp files in lisp/language were less than 0x4000
characters.

And, yes, we can estimate the actually necessarey
CHARBUF_SIZE as coding->src_bytes on decoding, and
coding->src_chars on encoding.

By the way, perhaps the most effective way for making
ENCODE_FILE faster and less memory-touching is to change
encode_file_name return FNAME when FNAME contains ASCII
only or file-name-coding-system is utf-8.

---
Kenichi Handa
handa@gnu.org



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-21 13:01     ` K. Handa
@ 2014-06-21 13:59       ` Eli Zaretskii
  2014-06-21 17:08         ` Stefan Monnier
  2014-06-22  9:22         ` K. Handa
  2014-06-21 15:19       ` David Kastrup
  1 sibling, 2 replies; 33+ messages in thread
From: Eli Zaretskii @ 2014-06-21 13:59 UTC (permalink / raw)
  To: K. Handa; +Cc: monnier, emacs-devel

> From: handa@gnu.org (K. Handa)
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> Date: Sat, 21 Jun 2014 22:01:56 +0900
> 
> In article <83lhsssq1b.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:
> 
> > That buffer is fixed in size, I don't know what.  Perhaps it's hard to
> > know in advance how much we will need.  I hope Handa-san could explain.
> 
> The reason of making it fixed size is that I was just lazy
> and didn't expect that decoder/encoder were called so
> frequently.  :-( The reaons of 0x4000 was that most of the
> elisp files in lisp/language were less than 0x4000
> characters.
> 
> And, yes, we can estimate the actually necessarey
> CHARBUF_SIZE as coding->src_bytes on decoding, and
> coding->src_chars on encoding.

Does the patch below look OK to you?  I tried it, but it seems to
cause trouble when byte-compiling: Emacs enters some kind of infloop.
Perhaps I misunderstood what you meant?

> By the way, perhaps the most effective way for making
> ENCODE_FILE faster and less memory-touching is to change
> encode_file_name return FNAME when FNAME contains ASCII
> only or file-name-coding-system is utf-8.

This would be a good improvement, as most platforms we care about use
UTF-8 for file-name encoding these days.  Could you please suggest
such changes?

TIA


=== modified file 'src/coding.c'
--- src/coding.c	2014-04-05 19:30:36 +0000
+++ src/coding.c	2014-06-21 13:47:39 +0000
@@ -7265,13 +7265,10 @@ produce_charset (struct coding_system *c
 		      coding->dst_object);
 }
 
-
-#define CHARBUF_SIZE 0x4000
-
-#define ALLOC_CONVERSION_WORK_AREA(coding)				\
+#define ALLOC_CONVERSION_WORK_AREA(coding, size)			\
   do {									\
-    coding->charbuf = SAFE_ALLOCA (CHARBUF_SIZE * sizeof (int));	\
-    coding->charbuf_size = CHARBUF_SIZE;				\
+    coding->charbuf = SAFE_ALLOCA ((size) * sizeof (int));		\
+    coding->charbuf_size = (size);					\
   } while (0)
 
 
@@ -7373,7 +7370,7 @@ decode_coding (struct coding_system *cod
   record_conversion_result (coding, CODING_RESULT_SUCCESS);
   coding->errors = 0;
 
-  ALLOC_CONVERSION_WORK_AREA (coding);
+  ALLOC_CONVERSION_WORK_AREA (coding, coding->src_bytes);
 
   attrs = CODING_ID_ATTRS (coding->id);
   translation_table = get_translation_table (attrs, 0, NULL);
@@ -7769,7 +7766,7 @@ encode_coding (struct coding_system *cod
   record_conversion_result (coding, CODING_RESULT_SUCCESS);
   coding->errors = 0;
 
-  ALLOC_CONVERSION_WORK_AREA (coding);
+  ALLOC_CONVERSION_WORK_AREA (coding, coding->src_chars);
 
   if (coding->encoder == encode_coding_ccl)
     {




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-21 13:01     ` K. Handa
  2014-06-21 13:59       ` Eli Zaretskii
@ 2014-06-21 15:19       ` David Kastrup
  1 sibling, 0 replies; 33+ messages in thread
From: David Kastrup @ 2014-06-21 15:19 UTC (permalink / raw)
  To: emacs-devel

handa@gnu.org (K. Handa) writes:

> In article <83lhsssq1b.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:
>
>> That buffer is fixed in size, I don't know what.  Perhaps it's hard to
>> know in advance how much we will need.  I hope Handa-san could explain.
>
> The reason of making it fixed size is that I was just lazy
> and didn't expect that decoder/encoder were called so
> frequently.  :-( The reaons of 0x4000 was that most of the
> elisp files in lisp/language were less than 0x4000
> characters.
>
> And, yes, we can estimate the actually necessarey
> CHARBUF_SIZE as coding->src_bytes on decoding, and
> coding->src_chars on encoding.

malloc/free pairings with lots of different sizes tend to be more work
for the heap management.  Of course, for alloca just using the minimum
is perfectly appropriate.

-- 
David Kastrup




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-21 13:59       ` Eli Zaretskii
@ 2014-06-21 17:08         ` Stefan Monnier
  2014-06-22  9:22         ` K. Handa
  1 sibling, 0 replies; 33+ messages in thread
From: Stefan Monnier @ 2014-06-21 17:08 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: K. Handa, emacs-devel

> This would be a good improvement, as most platforms we care about use
> UTF-8 for file-name encoding these days.  Could you please suggest
> such changes?

We can hope that the future will be "all utf-8" hence the need for
encoding/decoding will slowly disappear.  Not sure if we'll ever get
there, but indeed we can optimize the utf-8 case already.


        Stefan



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-21 13:59       ` Eli Zaretskii
  2014-06-21 17:08         ` Stefan Monnier
@ 2014-06-22  9:22         ` K. Handa
  2014-06-28 14:15           ` K. Handa
  1 sibling, 1 reply; 33+ messages in thread
From: K. Handa @ 2014-06-22  9:22 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, emacs-devel

In article <83egyitlbq.fsf@gnu.org>, Eli Zaretskii <eliz@gnu.org> writes:

> > And, yes, we can estimate the actually necessarey
> > CHARBUF_SIZE as coding->src_bytes on decoding, and
> > coding->src_chars on encoding.

> Does the patch below look OK to you?  I tried it, but it seems to
> cause trouble when byte-compiling: Emacs enters some kind of infloop.
> Perhaps I misunderstood what you meant?

I have not yet tested your patch, but I think it is better
to limit the maximum allocation size to the current
CHARBUF_SIZE, and also limit the mininum allocation size to
16 units or so.  The latter is because some of
encode_coding_XXXX need at least a few more slots left in
coding->charbuf.

> > By the way, perhaps the most effective way for making
> > ENCODE_FILE faster and less memory-touching is to change
> > encode_file_name return FNAME when FNAME contains ASCII
> > only or file-name-coding-system is utf-8.

> This would be a good improvement, as most platforms we care about use
> UTF-8 for file-name encoding these days.  Could you please suggest
> such changes?

Ok, I'll work on it.

---
Kenichi Handa
handa@gnu.org



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-22  9:22         ` K. Handa
@ 2014-06-28 14:15           ` K. Handa
  2014-06-28 14:38             ` Eli Zaretskii
  0 siblings, 1 reply; 33+ messages in thread
From: K. Handa @ 2014-06-28 14:15 UTC (permalink / raw)
  To: eliz; +Cc: monnier, emacs-devel

In article <871tuhti1h.fsf@gnu.org>, handa@gnu.org (K. Handa) writes:

> > Does the patch below look OK to you?  I tried it, but it seems to
> > cause trouble when byte-compiling: Emacs enters some kind of infloop.
> > Perhaps I misunderstood what you meant?

> I have not yet tested your patch,

I tested your patch, and confirmed the stuck.

> but I think it is better
> to limit the maximum allocation size to the current
> CHARBUF_SIZE, and also limit the mininum allocation size to
> 16 units or so.  The latter is because some of
> encode_coding_XXXX need at least a few more slots left in
> coding->charbuf.

So, along the above line, I intalled the attached patch
with which Emacs didn't stuck,

---
Kenichi Handa
handa@gnu.org

=== modified file 'src/coding.c'
--- src/coding.c	2014-06-25 12:11:08 +0000
+++ src/coding.c	2014-06-28 02:10:20 +0000
@@ -7265,13 +7265,16 @@
 		      coding->dst_object);
 }
 
+#define MAX_CHARBUF_SIZE 0x4000
+#define MIN_CHARBUF_SIZE 0x10
 
-#define CHARBUF_SIZE 0x4000
-
-#define ALLOC_CONVERSION_WORK_AREA(coding)				\
-  do {									\
-    coding->charbuf = SAFE_ALLOCA (CHARBUF_SIZE * sizeof (int));	\
-    coding->charbuf_size = CHARBUF_SIZE;				\
+#define ALLOC_CONVERSION_WORK_AREA(coding, size)		\
+  do {								\
+    int units = ((size) > MAX_CHARBUF_SIZE ? MAX_CHARBUF_SIZE	\
+		 : (size) < MIN_CHARBUF_SIZE ? MIN_CHARBUF_SIZE	\
+		 : size);					\
+    coding->charbuf = SAFE_ALLOCA ((units) * sizeof (int));	\
+    coding->charbuf_size = (units);				\
   } while (0)
 
 
@@ -7373,7 +7376,7 @@
   record_conversion_result (coding, CODING_RESULT_SUCCESS);
   coding->errors = 0;
 
-  ALLOC_CONVERSION_WORK_AREA (coding);
+  ALLOC_CONVERSION_WORK_AREA (coding, coding->src_bytes);
 
   attrs = CODING_ID_ATTRS (coding->id);
   translation_table = get_translation_table (attrs, 0, NULL);
@@ -7769,7 +7772,7 @@
   record_conversion_result (coding, CODING_RESULT_SUCCESS);
   coding->errors = 0;
 
-  ALLOC_CONVERSION_WORK_AREA (coding);
+  ALLOC_CONVERSION_WORK_AREA (coding, coding->src_chars);
 
   if (coding->encoder == encode_coding_ccl)
     {




^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: Enlarge MAX_ALLOCA?
  2014-06-28 14:15           ` K. Handa
@ 2014-06-28 14:38             ` Eli Zaretskii
  0 siblings, 0 replies; 33+ messages in thread
From: Eli Zaretskii @ 2014-06-28 14:38 UTC (permalink / raw)
  To: K. Handa; +Cc: monnier, emacs-devel

> From: handa@gnu.org (K. Handa)
> Cc: monnier@iro.umontreal.ca, emacs-devel@gnu.org
> Date: Sat, 28 Jun 2014 23:15:58 +0900
> 
> In article <871tuhti1h.fsf@gnu.org>, handa@gnu.org (K. Handa) writes:
> 
> > > Does the patch below look OK to you?  I tried it, but it seems to
> > > cause trouble when byte-compiling: Emacs enters some kind of infloop.
> > > Perhaps I misunderstood what you meant?
> 
> > I have not yet tested your patch,
> 
> I tested your patch, and confirmed the stuck.
> 
> > but I think it is better
> > to limit the maximum allocation size to the current
> > CHARBUF_SIZE, and also limit the mininum allocation size to
> > 16 units or so.  The latter is because some of
> > encode_coding_XXXX need at least a few more slots left in
> > coding->charbuf.
> 
> So, along the above line, I intalled the attached patch
> with which Emacs didn't stuck,

Thank you.



^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2014-06-28 14:38 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-19 16:02 Enlarge MAX_ALLOCA? Eli Zaretskii
2014-06-19 16:23 ` David Kastrup
2014-06-19 16:48   ` Eli Zaretskii
2014-06-19 17:04     ` David Kastrup
2014-06-19 17:14       ` Eli Zaretskii
2014-06-19 17:36         ` David Kastrup
2014-06-19 17:51           ` Eli Zaretskii
2014-06-19 18:21   ` Stefan Monnier
2014-06-19 21:13     ` David Kastrup
2014-06-20  7:10       ` Eli Zaretskii
2014-06-20  8:08         ` David Kastrup
2014-06-20  8:38     ` Dmitry Antipov
2014-06-20  8:56       ` Eli Zaretskii
2014-06-20  9:26       ` Andreas Schwab
2014-06-20  9:38         ` David Kastrup
2014-06-19 18:28 ` Stefan Monnier
2014-06-19 18:38   ` Eli Zaretskii
2014-06-19 20:37     ` Stefan Monnier
2014-06-20  7:08       ` Eli Zaretskii
2014-06-20 13:02         ` Stefan Monnier
2014-06-20 13:18           ` Eli Zaretskii
2014-06-20 14:43             ` Stefan Monnier
2014-06-20 14:50               ` Eli Zaretskii
2014-06-20 15:15               ` Herring, Davis
2014-06-20 15:44                 ` Dmitry Antipov
2014-06-20 18:36                   ` Eli Zaretskii
2014-06-21 13:01     ` K. Handa
2014-06-21 13:59       ` Eli Zaretskii
2014-06-21 17:08         ` Stefan Monnier
2014-06-22  9:22         ` K. Handa
2014-06-28 14:15           ` K. Handa
2014-06-28 14:38             ` Eli Zaretskii
2014-06-21 15:19       ` David Kastrup

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

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 read-only IMAP folder(s) and NNTP newsgroup(s).