Please test the merge of the concurrency branch

Please test the merge of the concurrency branch

[Eli Zaretskii]

I've pushed to the Emacs Git repository a new branch named
'test-concurrency'.  It's the result of merging the concurrency branch
with a week-old master, followed by fixing a few bugs I found (after
resolving the merge conflicts).

This is tested on Trisquel GNU/Linux (x86_64 build) and on MS-Windows
(x86 build with wide ints).  "Tested" here means that it builds,
passes the test suite as well as the master version on the same
machine, passes the new tests that test the concurrency features, and
the basic functionalities that are affected by the concurrency-related
changes -- subprocesses and networking -- work as well as they do on
master.

The new concurrency features are not thoroughly tested, so I expect
bugs to be reported when people start using them seriously.  I don't
think that should preclude us from landing this on master, though, as
long as the "usual", a.k.a. "single-threaded" Emacs works as well as
the master branch.

I'd appreciate if people who can afford it checked out this branch and
tested it on their systems, to make the probability of bad problems,
which I estimate as low, to be even lower, and thus to avoid the
danger of breaking master too hard.

If no serious issues are uncovered, this will land on master by
tomorrow evening (UTC).

Last, but not least: many thanks to Tom for designing and implementing
this, and to Ken for past merges with master which made my merge job
much simpler.

Re: Please test the merge of the concurrency branch

[Robert Marshall]

Eli Zaretskii <eliz@gnu.org> writes:

> I've pushed to the Emacs Git repository a new branch named
> 'test-concurrency'.  It's the result of merging the concurrency branch
> with a week-old master, followed by fixing a few bugs I found (after
> resolving the merge conflicts).
>
> This is tested on Trisquel GNU/Linux (x86_64 build) and on MS-Windows
> (x86 build with wide ints).  "Tested" here means that it builds,
> passes the test suite as well as the master version on the same
> machine, passes the new tests that test the concurrency features, and
> the basic functionalities that are affected by the concurrency-related
> changes -- subprocesses and networking -- work as well as they do on
> master.
>
> The new concurrency features are not thoroughly tested, so I expect
> bugs to be reported when people start using them seriously.  I don't
> think that should preclude us from landing this on master, though, as
> long as the "usual", a.k.a. "single-threaded" Emacs works as well as
> the master branch.
>

It fails to build for me

 CC       category.o
In file included from thread.h:22:0,
                 from lisp.h:797,
                 from category.c:33:
regex.h:438:9: error: unknown type name ‘ssize_t’
(and a few more complaints about ssize_t)

	Debian GNU/Linux 8.6 (jessie)

Robert

Re: Please test the merge of the concurrency branch

[Eli Zaretskii]

> From: Robert Marshall <robert.marshall@codethink.co.uk>
> Date: Fri, 09 Dec 2016 15:30:51 +0000
> 
>  CC       category.o
> In file included from thread.h:22:0,
>                  from lisp.h:797,
>                  from category.c:33:
> regex.h:438:9: error: unknown type name ‘ssize_t’
> (and a few more complaints about ssize_t)
> 
> 	Debian GNU/Linux 8.6 (jessie)

Thanks, please try again.

Re: Please test the merge of the concurrency branch

[Paul Eggert]

On 12/09/2016 08:26 AM, Eli Zaretskii wrote:
> Thanks, please try again.

On Fedora 24 x86-64:

   CC       sysdep.o
sysdep.c:61:1: error: function declaration isn’t a prototype 
[-Werror=strict-prototypes]
  extern int h_errno;
  ^~~~~~
In file included from sysdep.c:56:0:
sysdep.c:61:12: error: redundant redeclaration of ‘__h_errno_location’ 
[-Werror=redundant-decls]
  extern int h_errno;
             ^
/usr/include/netdb.h:61:13: note: previous declaration of 
‘__h_errno_location’ was here
  extern int *__h_errno_location (void) __THROW __attribute__ ((__const__));
              ^~~~~~~~~~~~~~~~~~

Re: Please test the merge of the concurrency branch

[Robert Marshall]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Robert Marshall <robert.marshall@codethink.co.uk>
>> Date: Fri, 09 Dec 2016 15:30:51 +0000
>> 
>>  CC       category.o
>> In file included from thread.h:22:0,
>>                  from lisp.h:797,
>>                  from category.c:33:
>> regex.h:438:9: error: unknown type name ‘ssize_t’
>> (and a few more complaints about ssize_t)
>> 
>> 	Debian GNU/Linux 8.6 (jessie)
>
> Thanks, please try again.

That now builds - thank you

Robert

Re: Please test the merge of the concurrency branch

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Fri, 9 Dec 2016 08:40:58 -0800
> 
>    CC       sysdep.o
> sysdep.c:61:1: error: function declaration isn’t a prototype 
> [-Werror=strict-prototypes]
>   extern int h_errno;
>   ^~~~~~
> In file included from sysdep.c:56:0:
> sysdep.c:61:12: error: redundant redeclaration of ‘__h_errno_location’ 
> [-Werror=redundant-decls]
>   extern int h_errno;
>              ^
> /usr/include/netdb.h:61:13: note: previous declaration of 
> ‘__h_errno_location’ was here
>   extern int *__h_errno_location (void) __THROW __attribute__ ((__const__));
>               ^~~~~~~~~~~~~~~~~~

Thanks, please try again.

Re: Please test the merge of the concurrency branch

[jpff]

Minor data point; build cleanly on OpenSuSE13.2 and started OK.  Not done 
much yet
==Jon ff

Re: Please test the merge of the concurrency branch

[Eli Zaretskii]

> Date: Fri, 9 Dec 2016 16:58:15 +0000 (GMT)
> From: jpff <jpff@codemist.co.uk>
> cc: Eli Zaretskii <eliz@gnu.org>, 
>     Robert Marshall <robert.marshall@codethink.co.uk>
> 
> Minor data point; build cleanly on OpenSuSE13.2 and started OK.  Not done 
> much yet

Thanks for the feedback.

Re: Please test the merge of the concurrency branch

[David Caldwell]

[-- Attachment #1: Type: text/plain, Size: 410 bytes --]

On 12/9/16 6:45 AM, Eli Zaretskii wrote:
> I'd appreciate if people who can afford it checked out this branch and
> tested it on their systems, to make the probability of bad problems,
> which I estimate as low, to be even lower, and thus to avoid the
> danger of breaking master too hard.

It built with no errors on Mac OS X 10.12.1. I launched it, opened a
file and nothing fell over.

-David



[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3819 bytes --]

Re: Please test the merge of the concurrency branch

[Karl Fogel]

Eli Zaretskii <eliz@gnu.org> writes:
>I've pushed to the Emacs Git repository a new branch named
>'test-concurrency'.  It's the result of merging the concurrency branch
>with a week-old master, followed by fixing a few bugs I found (after
>resolving the merge conflicts).
>
>This is tested on Trisquel GNU/Linux (x86_64 build) and on MS-Windows
>(x86 build with wide ints).  "Tested" here means that it builds,
>passes the test suite as well as the master version on the same
>machine, passes the new tests that test the concurrency features, and
>the basic functionalities that are affected by the concurrency-related
>changes -- subprocesses and networking -- work as well as they do on
>master.
>
>The new concurrency features are not thoroughly tested, so I expect
>bugs to be reported when people start using them seriously.  I don't
>think that should preclude us from landing this on master, though, as
>long as the "usual", a.k.a. "single-threaded" Emacs works as well as
>the master branch.
>
>I'd appreciate if people who can afford it checked out this branch and
>tested it on their systems, to make the probability of bad problems,
>which I estimate as low, to be even lower, and thus to avoid the
>danger of breaking master too hard.

Built revision ad03e7af8 to get "GNU Emacs 26.0.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.4) of 2016-12-09" (on a Debian GNU/Linux 'testing' distro, dist-upgraded as of this morning).

I used Emacs with no issues for a few hours, including running two shell buffers (often with simultaneous active command output), reading/writing email with Gnus, and doing various Org Mode things.

By the way, I also built an earlier revision (I don't remember the rev id now) from the branch, and used it for about an hour, likewise with no problems.  Then at some point I did 'git pull' and was surprised to get a merge -- surprised, because I had no local modifications.  I assume this means you or someone rebased the branch at some point?  Anyway, I just deleted the branch locally and refetched it, resulting in rev ad03e7af8, which I then built and used for the rest of the day.

Best regards,
-Karl

Re: Please test the merge of the concurrency branch

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 2423 bytes --]

It builds fine on my machine, and it doesn't seem to break Proof General (which makes heavy use of subprocesses), nor any of my packages' test suites :)

However, make check seems to fail:

make[2]: Entering directory '/build/emacs/master-clean/doc/misc'
make[2]: Nothing to be done for 'info'.
make[2]: Leaving directory '/build/emacs/master-clean/doc/misc'
  GEN      info/dir
make[1]: Leaving directory '/build/emacs/master-clean'
make -C test check
make[1]: Entering directory '/build/emacs/master-clean/test'
rm -f *.tmp
test ! -d ./automated
Makefile:159: recipe for target 'check-no-automated-subdir' failed
make[1]: *** [check-no-automated-subdir] Error 1
make[1]: Leaving directory '/build/emacs/master-clean/test'
Makefile:964: recipe for target 'check' failed
make: *** [check] Error 2

This is most likely a sign of my inability to run the test suite properly, of course.

Good job on the merge! Exciting stuff.

Cheers,
Clément.

On 2016-12-09 10:30, Robert Marshall wrote:
> Eli Zaretskii <eliz@gnu.org> writes:
> 
>> I've pushed to the Emacs Git repository a new branch named
>> 'test-concurrency'.  It's the result of merging the concurrency branch
>> with a week-old master, followed by fixing a few bugs I found (after
>> resolving the merge conflicts).
>>
>> This is tested on Trisquel GNU/Linux (x86_64 build) and on MS-Windows
>> (x86 build with wide ints).  "Tested" here means that it builds,
>> passes the test suite as well as the master version on the same
>> machine, passes the new tests that test the concurrency features, and
>> the basic functionalities that are affected by the concurrency-related
>> changes -- subprocesses and networking -- work as well as they do on
>> master.
>>
>> The new concurrency features are not thoroughly tested, so I expect
>> bugs to be reported when people start using them seriously.  I don't
>> think that should preclude us from landing this on master, though, as
>> long as the "usual", a.k.a. "single-threaded" Emacs works as well as
>> the master branch.
>>
> 
> It fails to build for me
> 
>  CC       category.o
> In file included from thread.h:22:0,
>                  from lisp.h:797,
>                  from category.c:33:
> regex.h:438:9: error: unknown type name ‘ssize_t’
> (and a few more complaints about ssize_t)
> 
> 	Debian GNU/Linux 8.6 (jessie)
> 
> Robert
> 
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Please test the merge of the concurrency branch

[Andrés Ramírez]

It builds fine on Archlinux.

--8<---------------cut here---------------start------------->8---
"GNU Emacs 26.0.50.1 (i686-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
 of 2016-12-09"
--8<---------------cut here---------------end--------------->8---

BR

Re: Please test the merge of the concurrency branch

[Eli Zaretskii]

> Cc: Tom Tromey <tom@tromey.com>, Ken Raeburn <raeburn@raeburn.org>
> From: David Caldwell <david@porkrind.org>
> Date: Fri, 9 Dec 2016 14:33:40 -0800
> 
> On 12/9/16 6:45 AM, Eli Zaretskii wrote:
> > I'd appreciate if people who can afford it checked out this branch and
> > tested it on their systems, to make the probability of bad problems,
> > which I estimate as low, to be even lower, and thus to avoid the
> > danger of breaking master too hard.
> 
> It built with no errors on Mac OS X 10.12.1. I launched it, opened a
> file and nothing fell over.

Great, thanks for testing.

Re: Please test the merge of the concurrency branch

[Eli Zaretskii]

> From: Karl Fogel <kfogel@red-bean.com>
> Cc: emacs-devel@gnu.org,  Tom Tromey <tom@tromey.com>,  Ken Raeburn <raeburn@raeburn.org>
> Date: Fri, 09 Dec 2016 16:58:24 -0600
> 
> >I'd appreciate if people who can afford it checked out this branch and
> >tested it on their systems, to make the probability of bad problems,
> >which I estimate as low, to be even lower, and thus to avoid the
> >danger of breaking master too hard.
> 
> Built revision ad03e7af8 to get "GNU Emacs 26.0.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.4) of 2016-12-09" (on a Debian GNU/Linux 'testing' distro, dist-upgraded as of this morning).
> 
> I used Emacs with no issues for a few hours, including running two shell buffers (often with simultaneous active command output), reading/writing email with Gnus, and doing various Org Mode things.

Thanks, this is good news.

> By the way, I also built an earlier revision (I don't remember the rev id now) from the branch, and used it for about an hour, likewise with no problems.  Then at some point I did 'git pull' and was surprised to get a merge -- surprised, because I had no local modifications.  I assume this means you or someone rebased the branch at some point?

Which branch? concurrency or test-concurrency?  These are different
branches; the latter is actually master with concurrency merged into
it (in de4624c), and with some followup commits to fix problems.

Regarding the original concurrency branch, Ken did a merge from master
around November last year.  Not sure if that explains what you saw.

If you checked out the test-concurrency branch, then there were no
rebases on it since it was pushed, a day before I announced it.  I
only added a few commits since then.

Re: Please test the merge of the concurrency branch

[Eli Zaretskii]

> From: Clément Pit--Claudel <clement.pit@gmail.com>
> Date: Fri, 9 Dec 2016 18:09:03 -0500
> 
> 
> It builds fine on my machine, and it doesn't seem to break Proof General (which makes heavy use of subprocesses), nor any of my packages' test suites :)

Excellent news!  Using Helm would be another good way of testing that.

> However, make check seems to fail:
> 
> make[2]: Entering directory '/build/emacs/master-clean/doc/misc'
> make[2]: Nothing to be done for 'info'.
> make[2]: Leaving directory '/build/emacs/master-clean/doc/misc'
>   GEN      info/dir
> make[1]: Leaving directory '/build/emacs/master-clean'
> make -C test check
> make[1]: Entering directory '/build/emacs/master-clean/test'
> rm -f *.tmp
> test ! -d ./automated
> Makefile:159: recipe for target 'check-no-automated-subdir' failed
> make[1]: *** [check-no-automated-subdir] Error 1

The error message was supposed to speak for itself :-(

The solution is to manually delete the 'automated' directory, it is a
left-over from some past merge from emacs-25.

This issue has nothing to do with real failures of the test suite,
it's just a precaution that the test suite takes to make sure no
spurious files and directories are present.

> Good job on the merge! Exciting stuff.

Thanks for testing it.

Re: Please test the merge of the concurrency branch

[Eli Zaretskii]

> From: Andrés Ramírez <sunshavi@fastmail.fm>
> Date: Fri, 09 Dec 2016 18:19:12 -0500
> 
> It builds fine on Archlinux.
> 
> --8<---------------cut here---------------start------------->8---
> "GNU Emacs 26.0.50.1 (i686-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
>  of 2016-12-09"
> --8<---------------cut here---------------end--------------->8---

Great, thanks.

Re: Please test the merge of the concurrency branch

[Andreas Politz]

It fails to build when CHECK_LISP_OBJECT_TYPE is defined.

../../../src/thread.c: In function ‘mark_one_thread’:
../../../src/thread.c:543:7: error: used struct type value where scalar is required
   if (thread->m_saved_last_thing_searched)
       ^~~~~~

Re: Please test the merge of the concurrency branch

[Eli Zaretskii]

> From: Andreas Politz <politza@hochschule-trier.de>
> Cc: emacs-devel@gnu.org
> Date: Sat, 10 Dec 2016 09:09:56 +0100
> 
> ../../../src/thread.c: In function ‘mark_one_thread’:
> ../../../src/thread.c:543:7: error: used struct type value where scalar is required
>    if (thread->m_saved_last_thing_searched)
>        ^~~~~~

Thanks, should be fixed now.

Re: Please test the merge of the concurrency branch

[Alan Third]

On Fri, Dec 09, 2016 at 02:33:40PM -0800, David Caldwell wrote:
> On 12/9/16 6:45 AM, Eli Zaretskii wrote:
> > I'd appreciate if people who can afford it checked out this branch and
> > tested it on their systems, to make the probability of bad problems,
> > which I estimate as low, to be even lower, and thus to avoid the
> > danger of breaking master too hard.
> 
> It built with no errors on Mac OS X 10.12.1. I launched it, opened a
> file and nothing fell over.

Seconded. I ran make check and it looks no different to master
on my machine.

-- 
Alan Third

Re: Please test the merge of the concurrency branch

[Eli Zaretskii]

> > It built with no errors on Mac OS X 10.12.1. I launched it, opened a
> > file and nothing fell over.
> 
> Seconded. I ran make check and it looks no different to master
> on my machine.

Great, thanks.

Re: Please test the merge of the concurrency branch

[Clément Pit--Claudel]

[-- Attachment #1.1: Type: text/plain, Size: 195 bytes --]

On 2016-12-10 02:14, Eli Zaretskii wrote:
> The error message was supposed to speak for itself :-(

Woops; sorry :/ It passes fine now.

Thanks again for all your hard work :)
Clément.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: Please test the merge of the concurrency branch

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: Clément Pit--Claudel <clement.pit@gmail.com>
> Date: Sat, 10 Dec 2016 10:05:13 -0500
> 
> On 2016-12-10 02:14, Eli Zaretskii wrote:
> > The error message was supposed to speak for itself :-(
> 
> Woops; sorry :/ It passes fine now.

Great, thanks for testing the branch.

Re: Please test the merge of the concurrency branch

[Joseph Mingrone]

[-- Attachment #1: Type: text/plain, Size: 1307 bytes --]

Hello Eli,

It built successfully on FreeBSD 11.0 AMD64 and seems to run fine with "normal" use after half an hour.

Are there are any specific tests that would be helpful?

Joseph

% gmake check # master branch
SUMMARY OF TEST RESULTS
-----------------------
Files examined: 127
Ran 1970 tests, 1946 results as expected, 4 unexpected, 20 skipped
3 files contained unexpected results:
  lisp/vc/vc-tests.log
  lisp/net/tramp-tests.log
  lisp/progmodes/flymake-tests.log
gmake[2]: *** [Makefile:187: check-doit] Error 1
gmake[2]: Leaving directory '/usr/home/jrm/scm/nm/emacs.git/test'
gmake[1]: *** [Makefile:168: check] Error 2
gmake[1]: Leaving directory '/usr/home/jrm/scm/nm/emacs.git/test'
gmake: *** [Makefile:964: check] Error 2

% gmake check # test-concurrency branch
.
.
.
SUMMARY OF TEST RESULTS
-----------------------
Files examined: 126
Ran 1934 tests, 1911 results as expected, 4 unexpected, 19 skipped
3 files contained unexpected results:
  lisp/vc/vc-tests.log
  lisp/net/tramp-tests.log
  lisp/progmodes/flymake-tests.log
gmake[2]: *** [Makefile:187: check-doit] Error 1
gmake[2]: Leaving directory '/usr/home/jrm/scm/nm/emacs.git/test'
gmake[1]: *** [Makefile:168: check] Error 2
gmake[1]: Leaving directory '/usr/home/jrm/scm/nm/emacs.git/test'
gmake: *** [Makefile:964: check] Error 2

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 930 bytes --]

Re: Please test the merge of the concurrency branch

[Achim Gratz]

Eli Zaretskii writes:
> I'd appreciate if people who can afford it checked out this branch and
> tested it on their systems, to make the probability of bad problems,
> which I estimate as low, to be even lower, and thus to avoid the
> danger of breaking master too hard.

Just built this Emacs on openSUSE/Tumbleweed for x86_64 and will use it
instead of master from now on.

The tests ran cleanly with one possible exception: network-stream-tests
hasn't finished yet and I see no network activity.  Killing the
remaining process yielded:

   FAILED   1/10  connect-to-tls-ipv4-nowait
gnutls-serv: Set static Diffie-Hellman parameters, consider --dhparams.
HTTP Server listening on IPv4 0.0.0.0 port 44332...done
HTTP Server listening on IPv6 :: port 44332...done

It seems that test fails on regular master as well, so I don't think
it's related to the changes in the test-concurrency branch.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for KORG EX-800 and Poly-800MkII V0.9:
http://Synth.Stromeko.net/Downloads.html#KorgSDada

Re: Please test the merge of the concurrency branch

[Filipe Silva]

[-- Attachment #1: Type: text/plain, Size: 1526 bytes --]

Eli,

just built emacs using the test-concurrency branch and all is going smooth
on latest OSx Sierra:

GNU Emacs 26.0.50.1 (x86_64-apple-darwin16.1.0, NS appkit-1504.60 Version
10.12.1 (Build 16B2555)) of 2016-12-10

I've just built it one hour ago. No issues up to this point.


best regards,

Ninrod.


On Sat, Dec 10, 2016 at 2:33 PM, Achim Gratz <Stromeko@nexgo.de> wrote:

> Eli Zaretskii writes:
> > I'd appreciate if people who can afford it checked out this branch and
> > tested it on their systems, to make the probability of bad problems,
> > which I estimate as low, to be even lower, and thus to avoid the
> > danger of breaking master too hard.
>
> Just built this Emacs on openSUSE/Tumbleweed for x86_64 and will use it
> instead of master from now on.
>
> The tests ran cleanly with one possible exception: network-stream-tests
> hasn't finished yet and I see no network activity.  Killing the
> remaining process yielded:
>
>    FAILED   1/10  connect-to-tls-ipv4-nowait
> gnutls-serv: Set static Diffie-Hellman parameters, consider --dhparams.
> HTTP Server listening on IPv4 0.0.0.0 port 44332...done
> HTTP Server listening on IPv6 :: port 44332...done
>
> It seems that test fails on regular master as well, so I don't think
> it's related to the changes in the test-concurrency branch.
>
>
> Regards,
> Achim.
> --
> +<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+
>
> SD adaptations for KORG EX-800 and Poly-800MkII V0.9:
> http://Synth.Stromeko.net/Downloads.html#KorgSDada
>
>
>

[-- Attachment #2: Type: text/html, Size: 2874 bytes --]

Concurrency has landed (was: Please test the merge of the concurrency branch)

[Eli Zaretskii]

> Date: Fri, 09 Dec 2016 16:45:22 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: Tom Tromey <tom@tromey.com>, Ken Raeburn <raeburn@raeburn.org>
> 
> If no serious issues are uncovered, this will land on master by
> tomorrow evening (UTC).

Now done.  Welcome to concurrency, Emacs!

Once again, many thanks to Tom for developing this.

Re: Please test the merge of the concurrency branch

[Eli Zaretskii]

> From: Joseph Mingrone <jrm@ftfl.ca>
> Cc: emacs-devel@gnu.org
> Date: Sat, 10 Dec 2016 12:05:18 -0400
> 
> It built successfully on FreeBSD 11.0 AMD64 and seems to run fine with "normal" use after half an hour.

Great, thanks.

> Are there are any specific tests that would be helpful?

No, nothing specific.  But since this now landed on master, just use
the master version in your routine work, and report any unusual
problems as bugs.

Thanks.

Re: Please test the merge of the concurrency branch

[Eli Zaretskii]

> From: Achim Gratz <Stromeko@nexgo.de>
> Date: Sat, 10 Dec 2016 17:33:47 +0100
> 
> Eli Zaretskii writes:
> > I'd appreciate if people who can afford it checked out this branch and
> > tested it on their systems, to make the probability of bad problems,
> > which I estimate as low, to be even lower, and thus to avoid the
> > danger of breaking master too hard.
> 
> Just built this Emacs on openSUSE/Tumbleweed for x86_64 and will use it
> instead of master from now on.

Great, thanks.  I have merged this feature with master about an hour
ago, so you can go back to master.

> The tests ran cleanly with one possible exception: network-stream-tests
> hasn't finished yet and I see no network activity.  Killing the
> remaining process yielded:
> 
>    FAILED   1/10  connect-to-tls-ipv4-nowait
> gnutls-serv: Set static Diffie-Hellman parameters, consider --dhparams.
> HTTP Server listening on IPv4 0.0.0.0 port 44332...done
> HTTP Server listening on IPv6 :: port 44332...done
> 
> It seems that test fails on regular master as well, so I don't think
> it's related to the changes in the test-concurrency branch.

Indeed, I see the same on the GNU/Linux machine where I tested this.
in my case, it dies because the address is already in use, whatever
that means.

Re: Please test the merge of the concurrency branch

[Eli Zaretskii]

> From: Filipe Silva <filipe.silva@gmail.com>
> Date: Sat, 10 Dec 2016 14:56:44 -0200
> Cc: emacs-devel@gnu.org
> 
> just built emacs using the test-concurrency branch and all is going smooth on latest OSx Sierra:
> 
> GNU Emacs 26.0.50.1 (x86_64-apple-darwin16.1.0, NS appkit-1504.60 Version 10.12.1 (Build 16B2555)) of
> 2016-12-10
> 
> I've just built it one hour ago. No issues up to this point. 

Thanks for testing.

Re: Concurrency has landed

[Eli Zaretskii]

> Date: Sat, 10 Dec 2016 19:36:10 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: tom@tromey.com
> 
> > If no serious issues are uncovered, this will land on master by
> > tomorrow evening (UTC).
> 
> Now done.  Welcome to concurrency, Emacs!

Paul,

Unfortunately, this effectively reverts your changes in 644fc17, which
removed the interpreter's byte stack.  The thread code saves and
restores the stack upon thread switch; since that removal gave me
trouble both during merge and while testing the results, I concluded
that the byte stack should be restored.

At the time, this conclusion made perfect sense to me.  However, in
hindsight, I can no longer convince myself the removal was justified.
So if you find that the byte stack can still be removed without
breaking the concurrency feature, feel free to do so, or just tell
what should be changed in the thread-related code, and I will do that.

Sorry about this.

Re: Concurrency has landed

[raman]

Just built master on Ubuntu Trusty --- built fines, and the result runs
happily with Emacspeak --
-- 

Re: Concurrency has landed

[Eli Zaretskii]

> From: raman <raman@google.com>
> Cc: Paul Eggert <eggert@cs.ucla.edu>,  emacs-devel@gnu.org
> Date: Sat, 10 Dec 2016 10:26:29 -0800
> 
> Just built master on Ubuntu Trusty --- built fines, and the result runs
> happily with Emacspeak --

Great, thanks.

Re: Concurrency has landed

[Paul Eggert]

Eli Zaretskii wrote:
> At the time, this conclusion made perfect sense to me.  However, in
> hindsight, I can no longer convince myself the removal was justified.
> So if you find that the byte stack can still be removed without
> breaking the concurrency feature, feel free to do so

I'll look into it at some point if I find the time.

If memory serves I removed the byte stack not only for performance reasons, but 
also because the byte stack implementation relies on using pointers to freed 
storage, which violates the C standard. Although we've been lucky that this 
works on Emacs platforms so far, our luck will run out once we get Emacs to work 
with gcc -fcheck-pointer-bounds.

Re: Please test the merge of the concurrency branch

[Ken Raeburn]

The branch builds & runs fine for me on Mac OS X 10.11.6 (i.e., one major release out of date).  Only tested with a few hours of normal code editing work locally and via TRAMP so far though.

Ken

Re: Please test the merge of the concurrency branch

[Eli Zaretskii]

> From: Ken Raeburn <raeburn@raeburn.org>
> Date: Sat, 10 Dec 2016 21:51:13 -0500
> Cc: emacs-devel@gnu.org,
>  Tom Tromey <tom@tromey.com>
> 
> The branch builds & runs fine for me on Mac OS X 10.11.6 (i.e., one major release out of date).  Only tested with a few hours of normal code editing work locally and via TRAMP so far though.

Thanks.  You can go back to master, now that concurrency landed there.

Re: Please test the merge of the concurrency branch

[Michael Albinus]

Ken Raeburn <raeburn@raeburn.org> writes:

> The branch builds & runs fine for me on Mac OS X 10.11.6 (i.e., one
> major release out of date).  Only tested with a few hours of normal
> code editing work locally and via TRAMP so far though.

When time permits I'll check how Tramp could profit from the
concurrency. First I'll release Tramp 2.3.1, so it might still take a
couple of weeks.

If people have proposals what to change in Tramp wrt concurrency, please
say so. Of course I know that, for example, file copying shall not block
further Emacs use, but I'm still uncertain how to integrate this. As of
today, all callers of `copy-file' and friends do expect, that the
copying has finished once the function returns.

> Ken

Best regards, Michael.

Re: Please test the merge of the concurrency branch

[Ken Raeburn]

On Dec 11, 2016, at 04:11, Michael Albinus <michael.albinus@gmx.de> wrote:

> Ken Raeburn <raeburn@raeburn.org> writes:
> 
>> The branch builds & runs fine for me on Mac OS X 10.11.6 (i.e., one
>> major release out of date).  Only tested with a few hours of normal
>> code editing work locally and via TRAMP so far though.
> 
> When time permits I'll check how Tramp could profit from the
> concurrency. First I'll release Tramp 2.3.1, so it might still take a
> couple of weeks.

There are still some basic bugs to work out…

> If people have proposals what to change in Tramp wrt concurrency, please
> say so. Of course I know that, for example, file copying shall not block
> further Emacs use, but I'm still uncertain how to integrate this. As of
> today, all callers of `copy-file' and friends do expect, that the
> copying has finished once the function returns.

Given that it’s an optional feature at this point (and not supported by every OS), I’d be careful in how you try to make use of it.

In fact, Tramp is one of the areas where you might want to consider whether some, ah, defensive changes might be wise first.  For example, if I have a couple of threads invoking find-file in a package of mine, and both invocations have Tramp filenames, and the Tramp code has blocking calls (e.g., waiting for subprocess output) that result in thread switching, both threads could be trying to mess around with the same Tramp data structures and subprocess communications, with messy results.  Unless we want to tell people “don’t do file operations in more than one thread”, or “don’t do Tramp operations in more than one thread” (in which case people doing file operations need to check whether they’re local files?), maybe Tramp should do some locking internally.  And what happens if I invoke “tramp-cleanup-all-connections” while a file-reading operation is in progress?

There are probably other parts of the Lisp library in a similar position, but the ones coming to mind offhand would probably generally be used in more direct fashion, whereas Tramp often hides under the normal file abstractions.

Ken

Tramp and concurrency (was: Please test the merge of the concurrency branch)

[Michael Albinus]

Ken Raeburn <raeburn@raeburn.org> writes:

>> If people have proposals what to change in Tramp wrt concurrency, please
>> say so. Of course I know that, for example, file copying shall not block
>> further Emacs use, but I'm still uncertain how to integrate this. As of
>> today, all callers of `copy-file' and friends do expect, that the
>> copying has finished once the function returns.
>
> Given that it’s an optional feature at this point (and not supported
> by every OS), I’d be careful in how you try to make use of it.
>
> In fact, Tramp is one of the areas where you might want to consider
> whether some, ah, defensive changes might be wise first.  For example,
> if I have a couple of threads invoking find-file in a package of mine,
> and both invocations have Tramp filenames, and the Tramp code has
> blocking calls (e.g., waiting for subprocess output) that result in
> thread switching, both threads could be trying to mess around with the
> same Tramp data structures and subprocess communications, with messy
> results.  Unless we want to tell people “don’t do file operations in
> more than one thread”, or “don’t do Tramp operations in more than one
> thread” (in which case people doing file operations need to check
> whether they’re local files?), maybe Tramp should do some locking
> internally.

Well, Tramp distinguishes between "inline methods" (all communication
goes over the same channel, like for "ssh"), and "external methods",
which invoke an own subprocess for data transfer, like for "scp". A
first candidate for concurrency would be external methods; switching
between processes happens there already.

And I agree that Tramp shall offer this as optional feature. This is
also a must, because Tramp supports also older Emacsen w/o concurrency.

> And what happens if I invoke “tramp-cleanup-all-connections” while a
> file-reading operation is in progress?

Don't know yet. Maybe all such functions in other threads must raise an
"interrupted" error, or so.

> Ken

Best regards, Michael.

Re: Please test the merge of the concurrency branch

[Eli Zaretskii]

> From: Ken Raeburn <raeburn@raeburn.org>
> Date: Sun, 11 Dec 2016 08:54:52 -0500
> Cc: Eli Zaretskii <eliz@gnu.org>,
>  Tom Tromey <tom@tromey.com>,
>  emacs-devel@gnu.org
> 
> For example, if I have a couple of threads invoking find-file in a package of mine, and both invocations have Tramp filenames, and the Tramp code has blocking calls (e.g., waiting for subprocess output) that result in thread switching, both threads could be trying to mess around with the same Tramp data structures and subprocess communications, with messy results.  Unless we want to tell people “don’t do file operations in more than one thread”, or “don’t do Tramp operations in more than one thread” (in which case people doing file operations need to check whether they’re local files?), maybe Tramp should do some locking internally.  And what happens if I invoke “tramp-cleanup-all-connections” while a file-reading operation is in progress?

There are synchronization facilities to take care of these.  And each
thread can have its local bindings of the same variables, which also
could help.

But yes, making a large package such as Tramp threads-aware will
probably take some work.

Re: Please test the merge of the concurrency branch

[Daimrod]

[-- Attachment #1.1: Type: text/plain, Size: 2125 bytes --]

Build & make check fine on OpenBSD (6.0-current).

The test `threads-io-switch' fails when runs in graphical emacs (gtk3).
Here is the backtrace (no debug symbol) :

#0  0x000000000054739a in SPECPDL_INDEX ()
#1  0x00000000005126c2 in handle_one_xevent ()
#2  0x0000000000514ffe in x_dispatch_event ()
#3  0x00000000005123fe in event_handler_gdk ()
#4  0x0000000284df0ca0 in gdk_event_apply_filters () from
/usr/local/lib/libgdk-3.so.2200.2
#5  0x0000000284df0e00 in _gdk_x11_display_queue_events () from
/usr/local/lib/libgdk-3.so.2200.2
#6  0x0000000284db98e8 in gdk_display_get_event () from
/usr/local/lib/libgdk-3.so.2200.2
#7  0x0000000284df0b06 in gdk_event_source_dispatch () from
/usr/local/lib/libgdk-3.so.2200.2
#8  0x000000021077965f in g_main_context_dispatch () from
/usr/local/lib/libglib-2.0.so.4200.4
#9  0x000000000069c458 in xg_select ()
#10 0x00000000006754db in really_call_select ()
#11 0x00000000005ce3c3 in flush_stack_call_func ()
#12 0x0000000000675570 in thread_select ()
#13 0x000000000064e4c1 in wait_reading_process_output ()
#14 0x000000000064ca59 in Faccept_process_output ()
#15 0x00000000005f449f in eval_sub ()
#16 0x00000000005f07b9 in Fprogn ()
#17 0x00000000005f1a7a in Fwhile ()
#18 0x00000000005f4002 in eval_sub ()
#19 0x00000000005f07b9 in Fprogn ()
#20 0x00000000005f4002 in eval_sub ()
#21 0x00000000005f090e in Fsetq ()
#22 0x00000000005f4002 in eval_sub ()
#23 0x00000000005f1f02 in Funwind_protect ()
#24 0x00000000005f4002 in eval_sub ()
#25 0x00000000005f068f in Fif ()
#26 0x00000000005f4002 in eval_sub ()
#27 0x00000000005f07b9 in Fprogn ()
#28 0x00000000005f19b2 in Flet ()
#29 0x00000000005f4002 in eval_sub ()
#30 0x00000000005f07b9 in Fprogn ()
#31 0x00000000005f19b2 in Flet ()
#32 0x00000000005f4002 in eval_sub ()
#33 0x00000000005f07b9 in Fprogn ()
#34 0x00000000005f6828 in funcall_lambda ()
#35 0x00000000005f5b95 in Ffuncall ()
#36 0x000000000063fbfe in exec_byte_code ()
#37 0x00000000005f652f in funcall_lambda ()
#38 0x00000000005f5adf in Ffuncall ()


I didn't look further yet.

Thanks for your work!


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 829 bytes --]

Re: Please test the merge of the concurrency branch

[Eli Zaretskii]

> From: Daimrod <daimrod@omecha.info>
> Date: Sun, 11 Dec 2016 19:30:39 +0100
> 
> Build & make check fine on OpenBSD (6.0-current).
> 
> The test `threads-io-switch' fails when runs in graphical emacs (gtk3).
> Here is the backtrace (no debug symbol) :

Do you see the same with the current master?

The backtrace looks similar to what Elias Mårtenson reported in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25172#5.

Re: Please test the merge of the concurrency branch

[Daimrod]

[-- Attachment #1.1: Type: text/plain, Size: 540 bytes --]



On 12/11/16 19:42, Eli Zaretskii wrote:
>> From: Daimrod <daimrod@omecha.info>
>> Date: Sun, 11 Dec 2016 19:30:39 +0100
>>
>> Build & make check fine on OpenBSD (6.0-current).
>>
>> The test `threads-io-switch' fails when runs in graphical emacs (gtk3).
>> Here is the backtrace (no debug symbol) :
> 
> Do you see the same with the current master?
> 
> The backtrace looks similar to what Elias Mårtenson reported in
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25172#5.
> 
> 

I am using master, commit 9970816.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 829 bytes --]

Re: Please test the merge of the concurrency branch

[Peter Wang]

[-- Attachment #1: Type: text/plain, Size: 738 bytes --]

To add another data point, the latest master builds and runs well for me
on NetBSD 7.0.1_PATCH amd64 (GCC (nb2 20150115) 4.8.4). I used it for a
while with normal editing and packages like Magit, W3m, SLIME and etc. No
problems so far.

On Sun, Dec 11, 2016 at 11:33 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Ken Raeburn <raeburn@raeburn.org>
> > Date: Sat, 10 Dec 2016 21:51:13 -0500
> > Cc: emacs-devel@gnu.org,
> >  Tom Tromey <tom@tromey.com>
> >
> > The branch builds & runs fine for me on Mac OS X 10.11.6 (i.e., one
> major release out of date).  Only tested with a few hours of normal code
> editing work locally and via TRAMP so far though.
>
> Thanks.  You can go back to master, now that concurrency landed there.
>
>

[-- Attachment #2: Type: text/html, Size: 1228 bytes --]

Re: Please test the merge of the concurrency branch

[Eli Zaretskii]

> From: Peter Wang <ptr.wang@gmail.com>
> Date: Mon, 12 Dec 2016 03:40:24 +0800
> Cc: tom@tromey.com, emacs-devel@gnu.org
> 
> To add another data point, the latest master builds and runs well for me on NetBSD 7.0.1_PATCH amd64
> (GCC (nb2 20150115) 4.8.4). I used it for a while with normal editing and packages like Magit, W3m, SLIME
> and etc. No problems so far.

Thanks.

Re: Please test the merge of the concurrency branch

[Karl Fogel]

Eli Zaretskii <eliz@gnu.org> writes:
>> By the way, I also built an earlier revision (I don't remember the
>> rev id now) from the branch, and used it for about an hour, likewise
>> with no problems.  Then at some point I did 'git pull' and was
>> surprised to get a merge -- surprised, because I had no local
>> modifications.  I assume this means you or someone rebased the
>> branch at some point?
>
>Which branch? concurrency or test-concurrency?  These are different
>branches; the latter is actually master with concurrency merged into
>it (in de4624c), and with some followup commits to fix problems.

'test-concurrency' only.

>If you checked out the test-concurrency branch, then there were no
>rebases on it since it was pushed, a day before I announced it.  I
>only added a few commits since then.

Hunh.  Well, I don't know what happened, in that case.  My local repository definitely didn't like two successive pulls a couple of hours apart, on the test-concurrency branch.  I did 'git reset --hard HEAD' after the first time and then re-pulled, to make sure I was really seeing what I thought I was seeing.  I had no local modifications or anything (if I had I would have stashed them).

Since there's no easy way to reconstruct or debug it now, and the branch has landed on master anyway, I guess I'll let this remain a mystery.

Re: Concurrency has landed

[Phillip Lord]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Fri, 09 Dec 2016 16:45:22 +0200
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: Tom Tromey <tom@tromey.com>, Ken Raeburn <raeburn@raeburn.org>
>> 
>> If no serious issues are uncovered, this will land on master by
>> tomorrow evening (UTC).
>
> Now done.  Welcome to concurrency, Emacs!
>
> Once again, many thanks to Tom for developing this.


Apologies if this has already been noted, but why is it
`thread--blocker', given that it's documented in the manual. Surely,
`thread-blocker'?

Phil

Re: Concurrency has landed

[Eli Zaretskii]

> From: phillip.lord@russet.org.uk (Phillip Lord)
> Cc: emacs-devel@gnu.org,  tom@tromey.com
> Date: Tue, 13 Dec 2016 12:28:57 +0000
> 
> Apologies if this has already been noted, but why is it
> `thread--blocker', given that it's documented in the manual. Surely,
> `thread-blocker'?

That was an explicit request from Stefan, back when this was
discussed.

Re: Concurrency has landed

[Daniel Colascione]

On Sat, Dec 10 2016, Paul Eggert wrote:
> Eli Zaretskii wrote:
>> At the time, this conclusion made perfect sense to me.  However, in
>> hindsight, I can no longer convince myself the removal was justified.
>> So if you find that the byte stack can still be removed without
>> breaking the concurrency feature, feel free to do so
>
> I'll look into it at some point if I find the time.
>
> If memory serves I removed the byte stack not only for performance
> reasons, but also because the byte stack implementation relies on
> using pointers to freed storage, which violates the C

How?

Re: Concurrency has landed

[Paul Eggert]

On 12/21/2016 08:52 PM, Daniel Colascione wrote:
>> reasons, but also because the byte stack implementation relies on
>> using pointers to freed storage, which violates the C
> How?

If memory serves, the code has several pointers p, q, r, ... into a 
memory region based at b that it wants to move. It then does the 
equivalent of 'b1 = realloc (b, newsize); p += b1-b; q += b1-b; r += 
b1-b; ...; b = b1;'. The C standard does not allow this: a program is 
not allowed to look at a pointer to freed storage (even if it does not 
dereference the pointer), which means the expression 'b1-b' has 
undefined behavior.

Possibly my memory is wrong and realloc was not involved. Regardless, 
the code in question does not work with -fcheck-pointer-bounds, and it's 
confusing to rely on pointers to freed storage, even if you don't 
dereference them and the code happens to work. Also, the byte stack 
isn't needed so we might as well remove it.

Re: Concurrency has landed

[Daniel Colascione]

On Thu, Dec 22 2016, Paul Eggert wrote:
> On 12/21/2016 08:52 PM, Daniel Colascione wrote:
>>> reasons, but also because the byte stack implementation relies on
>>> using pointers to freed storage, which violates the C
>> How?
>
> If memory serves, the code has several pointers p, q, r, ... into a
> memory region based at b that it wants to move. It then does the
> equivalent of 'b1 = realloc (b, newsize); p += b1-b; q += b1-b; r +=
> b1-b; ...; b = b1;'. The C standard does not allow this: a program is
> not allowed to look at a pointer to freed storage (even if it does not
> dereference the pointer), which means the expression 'b1-b' has
> undefined behavior.
>
> Possibly my memory is wrong and realloc was not involved. Regardless,
> the code in question does not work with -fcheck-pointer-bounds, and
> it's confusing to rely on pointers to freed storage, even if you don't
> dereference them and the code happens to work. Also, the byte stack
> isn't needed so we might as well remove it.

Thanks for the clarification: that's one of the less sensible things
about the C standard.  Pointers really ought to work like integers
whether or not they point to freed storage or utter nonsense.

We're not actually *using* the freed storage; that's what I was
worried about.

Re: Concurrency has landed

[Paul Eggert]

On 12/22/2016 11:32 AM, Daniel Colascione wrote:
> Pointers really ought to work like integers

Although that's true on most modern platforms, it's not necessarily true 
for platforms that use segmented memory, capabilities, and so forth. The 
"and so forth" includes GNU/Linux x86-64 when compiled with gcc 
-fcheck-pointer-bounds.

Re: Concurrency has landed

[Davis Herring]

> Thanks for the clarification: that's one of the less sensible things
> about the C standard.  Pointers really ought to work like integers
> whether or not they point to freed storage or utter nonsense.

For reference: C++ (which softens the use of such a pointer value to 
implementation-defined) comments that "Some implementations might define 
that copying an invalid pointer value causes a system-generated runtime 
fault.".

Davis

-- 
This product is sold by volume, not by mass.  If it appears too dense or 
too sparse, it is because mass-energy conversion has occurred during 
shipping.

Re: Concurrency has landed

[Stefan Monnier]

> Although that's true on most modern platforms, it's not necessarily true for
> platforms that use segmented memory, capabilities, and so forth.

While that's true for the usual "p + n goes over the limit", I really
can't imagine an architecture where "p1 - p2" is valid C code (i.e. both
p1 and p2 point inside the same object) and works before `free` yet it
doesn't give the same answer after `free`.


        Stefan


PS: Of course, I would not be surprised to hear that there's some
compiler out there which tries to be too clever for its own sake and
will go "oh, oh, this is not allowed by the C standard, so let's
compile this into utter nonsense just so SPEC runs 0.01% faster".

Re: Concurrency has landed

[Paul Eggert]

Stefan Monnier wrote:
> I really
> can't imagine an architecture where "p1 - p2" is valid C code (i.e. both
> p1 and p2 point inside the same object) and works before `free` yet it
> doesn't give the same answer after `free`.

That's not what the code in question does. p1 addresses a newly allocated 
object, and p2 is a dangling pointer that addresses the old (freed) object.

The problem with -fcheck-pointer-bounds does not occur when the program computes 
p1 - p2, as the program still gets a valid integer. The problem occurs later, 
when Emacs computes q + (p1 - p2), where q is a dangling pointer into the old 
object. On conventional architectures this has the same value as (q - p2) + p1, 
but that's not true with -fcheck-pointer-bounds, which attaches q's bounds 
(which are now invalid) to the resulting pointer.

Re: Concurrency has landed

[Stefan Monnier]

> The problem with -fcheck-pointer-bounds does not occur when the program
> computes p1 - p2, as the program still gets a valid integer. The problem
> occurs later, when Emacs computes q + (p1 - p2), where q is a dangling
> pointer into the old object.

Hmm... can you show in the code where we do that?  I'd expect the code
would do something like "newp2 = newp1 + (oldp2 - oldp1)", whereas you
seem to say that we do something more like "oldp2 = oldp1 + (newp2 -
newp1)", which would indeed be more risky (tho if oldp2 did exist before
the free, it should still work just fine even on odd architectures, but
at least I can see where an eager bounds-checker could complain).


        Stefan

Re: Concurrency has landed

[Paul Eggert]

Stefan Monnier wrote:
> can you show in the code where we do that?

Oh, sorry, my memory was wrong. I already removed the code with the problem in 
question, in commit b7f3a3055c92010afde318c1108b02e4424a3bac dated 2016-11-26. 
This code was in regex.c's EXTEND_BUFFER macro; it was a relic of an old hack 
that was removed from glibc and gnulib regex.c years ago, precisely because of 
(a precursor to) -fcheck-pointer-bounds.

The byte stack code does something similar, but does so in a safer way (this is 
in relocate_byte_stack). I plan to remove that too, though (again), since the 
byte stack isn't needed any more and slows things down a bit.