[Emacs-diffs] trunk r117941: Default to stack objects on non-GNU/Linux, non-DOS_NT platforms.

[Emacs-diffs] trunk r117941: Default to stack objects on non-GNU/Linux, non-DOS_NT platforms.

[Eli Zaretskii]

I don't understand this commit.

  . It was done in complete silence, without prior discussions wrt
    whether to make this the default.  (Yes, I suggested that, but no
    one replied, and we never discussed it.)

  . It arbitrarily excludes the native MS-Windows builds from this
    feature, for no good reasons: the 64-bit Windows build has no
    problems with it, and the problem discovered in the 32-bit build
    has a simple solution.  If you (Paul) didn't want to implement
    that solution yourself, you could have asked _before_ turning
    this on.

    In general, every feature that exists only on some platforms is a
    Bad Thing, as it introduces maintenance problems and in particular
    makes people who work on different platforms unable to usefully
    compare what they see and solve problems reported by others.

  . It does include the 32-bit Cygwin-w32 build which could
    potentially be affected by the same problem as the native Windows
    32-bit builds.

In sum, I don't understand these sneaky practices, and I wish they'd
stopped.  How many times do you have to be told to revert your commits
before you understand that this is not how we do things around here?

Re: [Emacs-diffs] trunk r117941: Default to stack objects on non-GNU/Linux, non-DOS_NT platforms.

[Dmitry Antipov]

On 09/25/2014 12:15 PM, Eli Zaretskii wrote:

> It was done in complete silence, without prior discussions wrt
> whether to make this the default.  (Yes, I suggested that, but no
> one replied, and we never discussed it.)

IMO 1) any discussions whether to make USE_STACK_LISP_OBJECTS the default
makes no sense until we know whether it's worth the complexities at all.
For the latter, we need a lot of feedback from users, preferably with
the very different usage patterns and workloads.  The simplest (and the
only reliable) method is to enable it by default and see what happens.
OTOH I'm not against making it opt-out, as you suggested.

> It arbitrarily excludes the native MS-Windows builds from this
> feature, for no good reasons: the 64-bit Windows build has no
> problems with it

IMO 2) responsible developer should not enable any code she/he can't
test.  If you are rather sure that it should work on 64-bit MS-Windows,
feel free to add this class of systems to an appropriate #ifdef (to be
honest, this is simpler and friendlier than writing such an indignant
e-mails).

> In general, every feature that exists only on some platforms is a
> Bad Thing, as it introduces maintenance problems and in particular
> makes people who work on different platforms unable to usefully
> compare what they see and solve problems reported by others.

USE_STACK_LISP_OBJECT is not such a feature.  It's rather under
construction and not yet populated to all (most?) platforms we aim
to support.  If you want to help with that, you're always welcome.

> In sum, I don't understand these sneaky practices, and I wish they'd
> stopped.

Sneaky?  I would call it too brave.

Dmitry

Re: [Emacs-diffs] trunk r117941: Default to stack objects on non-GNU/Linux, non-DOS_NT platforms.

[Eli Zaretskii]

> Date: Thu, 25 Sep 2014 13:51:44 +0400
> From: Dmitry Antipov <dmantipov@yandex.ru>
> CC: emacs-devel@gnu.org
> 
> On 09/25/2014 12:15 PM, Eli Zaretskii wrote:
> 
> > It was done in complete silence, without prior discussions wrt
> > whether to make this the default.  (Yes, I suggested that, but no
> > one replied, and we never discussed it.)
> 
> IMO 1) any discussions whether to make USE_STACK_LISP_OBJECTS the default
> makes no sense until we know whether it's worth the complexities at all.
> For the latter, we need a lot of feedback from users, preferably with
> the very different usage patterns and workloads.  The simplest (and the
> only reliable) method is to enable it by default and see what happens.
> OTOH I'm not against making it opt-out, as you suggested.

I suggested, and no one responded.  Ergo, no discussions.

You get more feedback if you enable the feature on all the platforms
that can support it.  Which is exactly my point.

> > It arbitrarily excludes the native MS-Windows builds from this
> > feature, for no good reasons: the 64-bit Windows build has no
> > problems with it
> 
> IMO 2) responsible developer should not enable any code she/he can't
> test.

You elided the part where I said that Paul could have asked others to
add whatever is necessary for 32-bit Windows.

> If you are rather sure that it should work on 64-bit MS-Windows

We tested that, so yes, I'm sure, as should be anyone else who reads
this list.

> > In general, every feature that exists only on some platforms is a
> > Bad Thing, as it introduces maintenance problems and in particular
> > makes people who work on different platforms unable to usefully
> > compare what they see and solve problems reported by others.
> 
> USE_STACK_LISP_OBJECT is not such a feature.  It's rather under
> construction and not yet populated to all (most?) platforms we aim
> to support.  If you want to help with that, you're always welcome.

You asked help for testing on Windows, and I did just that.  If you
need more help, you need but to ask.

> > In sum, I don't understand these sneaky practices, and I wish they'd
> > stopped.
> 
> Sneaky?  I would call it too brave.

Whatever you call it, it's not how we do things here.  Please don't
follow that bad example, and please don't get fascinated by it.

Re: [Emacs-diffs] trunk r117941: Default to stack objects on non-GNU/Linux, non-DOS_NT platforms.

[Stefan Monnier]

> IMO 1) any discussions whether to make USE_STACK_LISP_OBJECTS the default
> makes no sense until we know whether it's worth the complexities at all.
> For the latter, we need a lot of feedback from users, preferably with
> the very different usage patterns and workloads.

Actually, I can't see how workloads will affect the result.
What kind of feedback do you expect (other than bug-reports)?
If the only thing we expect are bug reports, then clearly we should
simply never enable this new code.

IIUC the point of this new code is to improve performance, but I can't
imagine any situation where the difference would be so fantastically big
that some user would send us feedback about it.

IOW the data needed to argue in favor of enabling this code is a few
benchmarks showing that performance is improved by some non-negligible
amount for some half-real use-cases.


        Stefan

Re: [Emacs-diffs] trunk r117941: Default to stack objects on non-GNU/Linux, non-DOS_NT platforms.

[Dmitry Antipov]

On 09/25/2014 04:52 PM, Stefan Monnier wrote:

> Actually, I can't see how workloads will affect the result.

Hm...for example, we don't allocate too much stack objects when byte-compile;
but we allocate substantial amount of them when attaching text properties in
a large buffer.

> What kind of feedback do you expect (other than bug-reports)?

If I had been aware of this, I would do everything myself.  Unfortunately
people uses Emacs in a lot of different ways :-).

> IIUC the point of this new code is to improve performance

Yes.  I should say that I've seen small but stable improvements before r117942,
but adding extra precautions against stack overflow makes them really negligible.
This means that we're going to an overengineered feature with no benefits.
IMO we should make it fast rather than commonly used.  For example, it's better
to avoid local_cons in loops (unless loop is bounded by small compile-time constant)
than check against MAX_ALLOCA each time; it may be worth trying to use
build_local_string only for a short unibyte compile-time string constants, etc.

Dmitry

Re: [Emacs-diffs] trunk r117941: Default to stack objects on non-GNU/Linux, non-DOS_NT platforms.

[Stefan Monnier]

>> IIUC the point of this new code is to improve performance
> Yes.  I should say that I've seen small but stable improvements before
> r117942,

It'd be good to see actual numbers.  I know that the performance
benefits for "average editing" is going to be negligible and I can live
with that, but I do want to see at least some real-use scenarios where
the benefit is tangible.

> but adding extra precautions against stack overflow makes
> them really negligible.  This means that we're going to an
> overengineered feature with no benefits.

I'm glad you feel that way.  When I saw this extra checking in
local_cons, I figured this is going to kill too much of the
performance gain.

> IMO we should make it fast rather than commonly used.

Right: the extra checks shouldn't fallback to Fcons but should
`abort' instead.


        Stefan

Re: [Emacs-diffs] trunk r117941: Default to stack objects on non-GNU/Linux, non-DOS_NT platforms.

[Paul Eggert]

On 09/25/2014 11:12 AM, Stefan Monnier wrote:
> I saw this extra checking in
> local_cons, I figured this is going to kill too much of the
> performance gain.

I agree that the performance gains are iffy, and would like to see some 
measurements here.

If it's any consolation, in some cases the new MAX_ALLOCA checking is 
optimized away.  For example, other_buffer_safely calls 
build_local_string twice, both with constant literals, and in both cases 
my compiler (GCC 4.9.1 x86-64) deduces that there's no need to check for 
MAX_ALLOCA, and generates code that creates the Lisp string on the stack 
unconditionally.

Re: [Emacs-diffs] trunk r117941: Default to stack objects on non-GNU/Linux, non-DOS_NT platforms.

[Eli Zaretskii]

> Date: Thu, 25 Sep 2014 11:15:02 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
>   . It arbitrarily excludes the native MS-Windows builds from this
>     feature, for no good reasons: the 64-bit Windows build has no
>     problems with it, and the problem discovered in the 32-bit build
>     has a simple solution.  If you (Paul) didn't want to implement
>     that solution yourself, you could have asked _before_ turning
>     this on.

As of trunk revision 117945, Emacs defaults to stack objects on DOS_NT
platforms as well.  If you have any problems with that, try rebuilding
after editing src/lisp.h to set USE_STACK_LISP_OBJECTS to false, and
if that makes the problems you see disappear, please report the
details via "M-x report-emacs-bug RET".

TIA

Re: [Emacs-diffs] trunk r117941: Default to stack objects on non-GNU/Linux, non-DOS_NT platforms.

[Paul Eggert]

Eli Zaretskii wrote:
> I don't understand these sneaky practices, and I wish they'd
> stopped.  How many times do you have to be told to revert your commits
> before you understand that this is not how we do things around here?

The issue was briefly discussed, there were problems with MS-Windows 
that I do not follow and cannot easily test, Dmitry played it safe and 
left MS-Windows alone, and I followed his lead.  It was reasonable for 
us to be cautious about enabling USE_STACK_LISP_OBJECTS on an untested 
and potentially-problematic platform.  And it was reasonable for you to 
enable USE_STACK_LISP_OBJECTS on MS-Windows once you installed further 
changes to fix the problems and had tested the result.  There was 
nothing sneaky about any of this, and the above-quoted remarks are 
unwarranted.

By the way, I share Stefan's concern that the performance improvements 
of USE_STACK_LISP_OBJECTS haven't been adequately demonstrated.

Re: [Emacs-diffs] trunk r117941: Default to stack objects on non-GNU/Linux, non-DOS_NT platforms.

[Eli Zaretskii]

> Date: Thu, 25 Sep 2014 09:06:42 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: emacs-devel@gnu.org
> 
> Eli Zaretskii wrote:
> > I don't understand these sneaky practices, and I wish they'd
> > stopped.  How many times do you have to be told to revert your commits
> > before you understand that this is not how we do things around here?
> 
> The issue was briefly discussed, there were problems with MS-Windows 
> that I do not follow and cannot easily test, Dmitry played it safe and 
> left MS-Windows alone, and I followed his lead.  It was reasonable for 
> us to be cautious about enabling USE_STACK_LISP_OBJECTS on an untested 
> and potentially-problematic platform.  And it was reasonable for you to 
> enable USE_STACK_LISP_OBJECTS on MS-Windows once you installed further 
> changes to fix the problems and had tested the result.

The point is that whether to turn this on by default and for which
platforms was never discussed.  Dmitry asked for help in testing that
on Windows, and I provided my conclusions about that.  Another user
reported problems with clang.  That's it; no further discussions were
conducted.  What you say above does not (and cannot) refute this basic
fact.

This is not how this should be done.  We should discuss first, arrive
at some conclusion, and only then act.  This is a team project, and no
single one of us can hope to grasp all the effects and consequences of
any serious changes such as this one.

> the above-quoted remarks are unwarranted.

On the contrary, they were well deserved.

> By the way, I share Stefan's concern that the performance improvements 
> of USE_STACK_LISP_OBJECTS haven't been adequately demonstrated.

So do I, but that's another issue, unrelated to what I protested
about.

Re: [Emacs-diffs] trunk r117941: Default to stack objects on non-GNU/Linux, non-DOS_NT platforms.

[Paul Eggert]

On 09/25/2014 09:48 AM, Eli Zaretskii wrote:
> This is a team project

Of course, and members of a team should work to support each other.  
There was no reason to use language like "sneaky" and "How many times do 
you have to be told to revert your commits".Language like that is 
off-putting; it dissuades other potential contributors to the project, 
something that's far more important than this minor technical issue.  
Please try to be more temperate in the future.

Re: [Emacs-diffs] trunk r117941: Default to stack objects on non-GNU/Linux, non-DOS_NT platforms.

[Eli Zaretskii]

> Date: Thu, 25 Sep 2014 11:46:04 -0700
> From: Paul Eggert <eggert@cs.ucla.edu>
> CC: emacs-devel@gnu.org
> 
> Please try to be more temperate in the future.

Please try to be more of a team-player in the future.

Re: [Emacs-diffs] trunk r117941: Default to stack objects on non-GNU/Linux, non-DOS_NT platforms.

[Stefan Monnier]

>> Please try to be more temperate in the future.
> Please try to be more of a team-player in the future.

Right, let's all try to be more temperate and better team players.


        Stefan