Skip to content

Commit

Permalink
Trac #10295: Upgrade and optimize pexpect
Browse files Browse the repository at this point in the history
We use pexpect version 2.0. Shouldn't we upgrade to the current version
4.0.1?

See also #10294.

'''New upstream tarballs''':
* https://pypi.python.org/packages/source/p/pexpect/pexpect-4.0.1.tar.gz
* https://pypi.python.org/packages/source/p/ptyprocess/ptyprocess-0.5.ta
r.gz
* http://www.math-cs.gordon.edu/~kcrisman/sagenb-0.11.6.1.tar

'''Patches included and submitted upstream''':
* [pexpect/pexpect#291]
* [pexpect/pexpect#303]
* [pexpect/pexpect#304]
* [pexpect/pexpect#307]

'''Note''' that this branch ''includes'' #19616.

-----

'''Throughput timings''' (best out of 5):

pexpect 2.0 upstream:
{{{
sage: %time _ = str(gp("2^2^22"))
CPU times: user 184 ms, sys: 4 ms, total: 188 ms
Wall time: 384 ms
}}}

pexpect 4.0.1 upstream:
{{{
sage: %time _ = str(gp("2^2^22"))
CPU times: user 208 ms, sys: 4 ms, total: 212 ms
Wall time: 405 ms
}}}

pexpect 4.0.1 + Sage patches:
{{{
sage: %time _ = str(gp("2^2^22"))
CPU times: user 9 ms, sys: 7 ms, total: 16 ms
Wall time: 209 ms
}}}

-----

'''Latency timings''' (best out of 5):

pexpect 2.0 upstream:
{{{
sage: gp(1);
sage: %time _ = [gp(i) for i in range(10^4)]
CPU times: user 2.96 s, sys: 855 ms, total: 3.82 s
Wall time: 4.52 s
}}}

pexpect 4.0.1 upstream:
{{{
sage: gp(1);
sage: %time _ = [gp(i) for i in range(10^4)]
CPU times: user 2.61 s, sys: 758 ms, total: 3.37 s
Wall time: 3.48 s
}}}

pexpect 4.0.1 + Sage patches:
{{{
sage: gp(1);
sage: %time _ = [gp(i) for i in range(10^4)]
CPU times: user 2.3 s, sys: 999 ms, total: 3.3 s
Wall time: 3.4 s
}}}

URL: http://trac.sagemath.org/10295
Reported by: SimonKing
Ticket author(s): François Bissey, Bill Page, Jeroen Demeyer
Reviewer(s): Jeroen Demeyer, Bill Page
  • Loading branch information
Release Manager authored and vbraun committed Dec 21, 2015
2 parents 3524b13 + 9351ccb commit c12ef20
Show file tree
Hide file tree
Showing 42 changed files with 325 additions and 188 deletions.
39 changes: 4 additions & 35 deletions build/pkgs/pexpect/SPKG.txt
Original file line number Diff line number Diff line change
Expand Up @@ -7,46 +7,15 @@ controlling them; and responding to expected patterns in their output.

== License ==

Python Software Foundation License
ISC license: http://opensource.org/licenses/isc-license.txt
This license is approved by the OSI and FSF as GPL-compatible.

== Upstream Contact ==

http://pexpect.readthedocs.org/en/stable/
https://github.com/pexpect/pexpect

== Dependencies ==

* GNU patch
* Python

== Special Update/Build Instructions ==

* pexpect-2.1 is shockingly slow for Sage (especially in the notebook)
and I can't figure out why. So we're sticking with pexpect-2.0 for
now (which works fine for us).

Patches:
* del.patch: put a try/except inside __del__, to get rid of lots of
pointless warning messages
* pexpect.py-isdir_bug_fix.patch: fixed bug in pexpect.py where it
tried to run directories
* env.patch: add env parameter to spawn.__init__() to run a command
with custom environment variables (as in pexpect-2.3).

== Changelog ==

=== pexpect-2.0.p6 (Bradly Schlenker, 10 May 2014) ===
* Trac #15178: Fixed pexpect.py-isdir_bug_fix.patch. (mislabeled variable)

=== pexpect-2.0.p5 (Jeroen Demeyer, 13 January 2012) ===
* Trac #12221: add env.patch to add env parameter to spawn.__init__()
* Restore upstream sources, use patch for patching

=== pexpect-2.0.p4 ===
* undocumented

=== pexpect-2.0.p3 (Michael Abshoff, January 28th, 2009) ===
* further clean up SPKG.txt
* add .hgignore

=== pexpect-2.0.p2 (William Stein, January 23rd, 2009) ===
* created proper SPKG.txt
* fixed bug in pexpect where it tried to run directories
8 changes: 4 additions & 4 deletions build/pkgs/pexpect/checksums.ini
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
tarball=pexpect-VERSION.tar.bz2
sha1=c4167a668e75b36a9e698c450627dac29ced6320
md5=d9a3e113ed147dcee8f89962a8dccd43
cksum=3775914086
tarball=pexpect-VERSION.tar.gz
sha1=fcaa594dd1ea97f1200b6b2819811231f69ec186
md5=056df81e6ca7081f1015b4b147b977b7
cksum=762875584
2 changes: 1 addition & 1 deletion build/pkgs/pexpect/dependencies
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
$(INST)/$(PYTHON)
$(INST)/$(PYTHON) $(INST)/$(PTYPROCESS)

----------
All lines of this file are ignored except the first.
Expand Down
2 changes: 1 addition & 1 deletion build/pkgs/pexpect/package-version.txt
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2.0.p6
4.0.1
15 changes: 0 additions & 15 deletions build/pkgs/pexpect/patches/del.patch

This file was deleted.

32 changes: 0 additions & 32 deletions build/pkgs/pexpect/patches/env.patch

This file was deleted.

43 changes: 43 additions & 0 deletions build/pkgs/pexpect/patches/none_delaybeforesend.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
commit 617d25553196aa58a3d8a2987008c22c73a86b19
Author: Jeroen Demeyer <[email protected]>
Date: Wed Dec 9 22:46:54 2015 +0100

Allow delaybeforesend=None to skip the sleep completely

diff --git a/doc/commonissues.rst b/doc/commonissues.rst
index d3aa9d0..f60085e 100644
--- a/doc/commonissues.rst
+++ b/doc/commonissues.rst
@@ -47,7 +47,7 @@ not be an issue for most users. For some applications you might with to turn it
off::

child = pexpect.spawn ("ssh [email protected]")
- child.delaybeforesend = 0
+ child.delaybeforesend = None

Truncated output just before child exits
----------------------------------------
diff --git a/pexpect/pty_spawn.py b/pexpect/pty_spawn.py
index 299016c..1e9a032 100644
--- a/pexpect/pty_spawn.py
+++ b/pexpect/pty_spawn.py
@@ -144,8 +144,7 @@ class spawn(SpawnBase):
many users that I decided that the default pexpect behavior should be
to sleep just before writing to the child application. 1/20th of a
second (50 ms) seems to be enough to clear up the problem. You can set
- delaybeforesend to 0 to return to the old behavior. Most Linux machines
- don't like this to be below 0.03. I don't know why.
+ delaybeforesend to None to return to the old behavior.

Note that spawn is clever about finding commands on your path.
It uses the same logic that "which" uses to find executables.
@@ -511,7 +510,8 @@ class spawn(SpawnBase):
>>> bash.sendline('x' * 5000)
'''

- time.sleep(self.delaybeforesend)
+ if self.delaybeforesend is not None:
+ time.sleep(self.delaybeforesend)

s = self._coerce_send_string(s)
self._log(s, 'send')
38 changes: 38 additions & 0 deletions build/pkgs/pexpect/patches/pexpect-4.0.1_superfluous_sleep.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
Patch taken from https://github.com/pexpect/pexpect/pull/291

diff --git a/pexpect/expect.py b/pexpect/expect.py
index 6fde9e8..1c7a163 100644
--- a/pexpect/expect.py
+++ b/pexpect/expect.py
@@ -95,7 +95,8 @@ class Expecter(object):
return self.timeout()
# Still have time left, so read more data
incoming = spawn.read_nonblocking(spawn.maxread, timeout)
- time.sleep(0.0001)
+ if self.spawn.delayafterread is not None:
+ time.sleep(self.spawn.delayafterread)
if timeout is not None:
timeout = end_time - time.time()
except EOF as e:
@@ -294,4 +295,4 @@ class searcher_re(object):
self.start = first_match
self.match = the_match
self.end = self.match.end()
- return best_index
\ No newline at end of file
+ return best_index
diff --git a/pexpect/spawnbase.py b/pexpect/spawnbase.py
index 0518d83..4664884 100644
--- a/pexpect/spawnbase.py
+++ b/pexpect/spawnbase.py
@@ -70,6 +70,10 @@ class SpawnBase(object):
# Used by terminate() to give kernel time to update process status.
# Time in seconds.
self.delayafterterminate = 0.1
+ # After each call to read_nonblocking(), pexpect releases the GIL
+ # through a time.sleep(0.0001) call by default since version 2.1.
+ # When set as value 'None', the old 2.0 behavior is restored.
+ self.delayafterread = 0.0001
self.softspace = False
self.name = '<' + repr(self) + '>'
self.closed = True
20 changes: 0 additions & 20 deletions build/pkgs/pexpect/patches/pexpect.py-isdir_bug_fix.patch

This file was deleted.

89 changes: 89 additions & 0 deletions build/pkgs/pexpect/patches/read_more_bytes.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
commit 3446e6d160c2804d6cd8709099a281011207cd29
Author: Jeroen Demeyer <[email protected]>
Date: Wed Dec 9 15:12:38 2015 +0100

#304: Read more bytes in read_nonblocking()

diff --git a/pexpect/pty_spawn.py b/pexpect/pty_spawn.py
index 299016c..c800c7f 100644
--- a/pexpect/pty_spawn.py
+++ b/pexpect/pty_spawn.py
@@ -415,14 +415,36 @@ class spawn(SpawnBase):
available right away then one character will be returned immediately.
It will not wait for 30 seconds for another 99 characters to come in.

+ On the other hand, if there are bytes available to read immediately,
+ all those bytes will be read (up to the buffer size). So, if the
+ buffer size is 1 megabyte and there is 1 megabyte of data available
+ to read, the buffer will be filled, regardless of timeout.
+
This is a wrapper around os.read(). It uses select.select() to
implement the timeout. '''

if self.closed:
raise ValueError('I/O operation on closed file.')

- if timeout == -1:
- timeout = self.timeout
+ # If there is data available to read right now, read as much as
+ # we can. We do this to increase performance if there are a lot
+ # of bytes to be read. This also avoids calling isalive() too
+ # often. See also:
+ # * https://github.com/pexpect/pexpect/pull/304
+ # * http://trac.sagemath.org/ticket/10295
+ r, w, e = self.__select([self.child_fd], [], [], 0)
+ if r:
+ incoming = super(spawn, self).read_nonblocking(size)
+ while len(incoming) < size:
+ r, w, e = self.__select([self.child_fd], [], [], 0)
+ if not r:
+ break
+ try:
+ incoming += super(spawn, self).read_nonblocking(size - len(incoming))
+ except EOF:
+ # Ignore EOF, just return the bytes we got so far.
+ break
+ return incoming

# Note that some systems such as Solaris do not give an EOF when
# the child dies. In fact, you can still try to read
@@ -430,11 +452,7 @@ class spawn(SpawnBase):
# For this case, I test isalive() before doing any reading.
# If isalive() is false, then I pretend that this is the same as EOF.
if not self.isalive():
- # timeout of 0 means "poll"
- r, w, e = self.__select([self.child_fd], [], [], 0)
- if not r:
- self.flag_eof = True
- raise EOF('End Of File (EOF). Braindead platform.')
+ raise EOF('End Of File (EOF). Braindead platform.')
elif self.__irix_hack:
# Irix takes a long time before it realizes a child was terminated.
# FIXME So does this mean Irix systems are forced to always have
@@ -444,7 +462,13 @@ class spawn(SpawnBase):
self.flag_eof = True
raise EOF('End Of File (EOF). Slow platform.')

- r, w, e = self.__select([self.child_fd], [], [], timeout)
+ # Wait with timeout until data is available to read.
+ # Since we already called select() with timeout=0, we don't
+ # do that again if timeout == 0.
+ if timeout == -1:
+ timeout = self.timeout
+ if timeout != 0:
+ r, w, e = self.__select([self.child_fd], [], [], timeout)

if not r:
if not self.isalive():
@@ -456,10 +480,7 @@ class spawn(SpawnBase):
else:
raise TIMEOUT('Timeout exceeded.')

- if self.child_fd in r:
- return super(spawn, self).read_nonblocking(size)
-
- raise ExceptionPexpect('Reached an unexpected state.') # pragma: no cover
+ return super(spawn, self).read_nonblocking(size)

def write(self, s):
'''This is similar to send() except that there is no return value.
27 changes: 27 additions & 0 deletions build/pkgs/pexpect/patches/spawnpty.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
Allow overriding the call to PtyProcess.spawn()
See https://github.com/pexpect/pexpect/pull/303/

--- a/pexpect/pty_spawn.py 2015-12-04 10:50:32.000000000 +0100
+++ b/pexpect/pty_spawn.py 2015-12-04 10:55:13.131695441 +0100
@@ -290,8 +290,8 @@
self.args = [a if isinstance(a, bytes) else a.encode(self.encoding)
for a in self.args]

- self.ptyproc = ptyprocess.PtyProcess.spawn(self.args, env=self.env,
- cwd=self.cwd, **kwargs)
+ self.ptyproc = self.spawnpty(self.args, env=self.env,
+ cwd=self.cwd, **kwargs)

self.pid = self.ptyproc.pid
self.child_fd = self.ptyproc.fd
@@ -300,6 +300,10 @@
self.terminated = False
self.closed = False

+ def spawnpty(self, args, **kwargs):
+ '''Spawn a pty and return an instance of PtyProcess.'''
+ return ptyprocess.PtyProcess.spawn(args, **kwargs)
+
def close(self, force=True):
'''This closes the connection with the child application. Note that
calling close() more than once is valid. This emulates standard Python
6 changes: 1 addition & 5 deletions build/pkgs/pexpect/spkg-install
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,14 @@ cd src

# Apply patches
for patch in ../patches/*.patch; do
[ -r "$patch" ] || continue # Skip non-existing or non-readable patches
patch -p1 <"$patch"
if [ $? -ne 0 ]; then
echo >&2 "Error applying '$patch'"
exit 1
fi
done

# use 2to3 if we are running on python3
if python -c 'import sys; sys.exit(sys.version_info.major != 3)'; then
2to3 -n -w --no-diffs pexpect.py
fi

# On some systems pexpect doesn't upgrade correctly without
# first removing the old version.
rm -f "$SAGE_LOCAL"/lib/python*/site-packages/pexpect.*
Expand Down
25 changes: 25 additions & 0 deletions build/pkgs/ptyprocess/SPKG.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
= ptyprocess =

== Description ==

Launch a subprocess in a pseudo terminal (pty), and interact with both the
process and its pty.

Sometimes, piping stdin and stdout is not enough. There might be a password
prompt that doesn't read from stdin, output that changes when it's going to a
pipe rather than a terminal, or curses-style interfaces that rely on a terminal.
If you need to automate these things, running the process in a pseudo terminal
(pty) is the answer.

== License ==

Ptyprocess is under the ISC license, as code derived from Pexpect.
http://opensource.org/licenses/ISC

== Upstream Contact ==

https://github.com/pexpect/ptyprocess

== Dependencies ==

* Python
4 changes: 4 additions & 0 deletions build/pkgs/ptyprocess/checksums.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
tarball=ptyprocess-VERSION.tar.gz
sha1=bf0c6664249a738c2b2a7251f23a7b14c5088150
md5=0d1ecfba622cb4e35ee157c38de18eae
cksum=1479514456
5 changes: 5 additions & 0 deletions build/pkgs/ptyprocess/dependencies
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
$(INST)/$(PYTHON)

----------
All lines of this file are ignored except the first.
It is copied by SAGE_ROOT/build/make/install into SAGE_ROOT/build/make/Makefile.
1 change: 1 addition & 0 deletions build/pkgs/ptyprocess/package-version.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
0.5
Loading

0 comments on commit c12ef20

Please sign in to comment.