bytearray() is using a less efficient implementation of split() than its similar friends bytes and unicode.

I added a couple extra checks to return early in {bytes,unicode} split().
 - if length is 0
 - if left strip removed all bytes

Looking at just how similar these 3 implementations are I feel that we could also unify/generalize them into one generic helper implementation and leave the object marshaling up to the type specific methods.

+    if (len == 0) {
+        if (PyByteArray_CheckExact(self)) {
+            Py_INCREF(self);
+            return (PyObject *)self;
+        }
+        return PyByteArray_FromStringAndSize(NULL, 0);
+    }

This looks like a dubious micro-optimization. If len == 0,
all loops will exit early anyway (same for similar snippets in bytesobject.c and unicodeobject.c).

+    if (i == 0 && j == len && PyByteArray_CheckExact(self)) {
+        Py_INCREF(self);
+        return (PyObject *)self;
+    }

bytearray objects are mutable, so you can't return the same object without breaking expected semantics. e.g.:

>>> a = bytearray()
>>> b = a.strip()
>>> b.append(49)
>>> a
bytearray(b'')
>>> b
bytearray(b'1')

> diff -r b5ccdf7c032a Python/bltinmodule.c
> Binary file Python/bltinmodule.c has changed

Uh, what's this? bltinmodule.c shouldn't be considered a binary file.
It probably means you added a NUL byte in it by mistake.

> Looking at just how similar these 3 implementations are I feel that we 
> could also unify/generalize them into one generic helper implementation 
> and leave the object marshaling up to the type specific methods.

You could put it in Objects/stringlib.

You are right about those lines in particular; 'dubious' as you say.
Though, the point overall was that the general implementation was noticeably
faster (regardless of those smaller changes).

However, I do think that the single check for len == 0 saves time
particularly
from do_xstrip where we call _getbuffer().

I see. I must have been trigger happy.

> > diff -r b5ccdf7c032a Python/bltinmodule.c
> > Binary file Python/bltinmodule.c has changed
>
> Uh, what's this? bltinmodule.c shouldn't be considered a binary file.
> It probably means you added a NUL byte in it by mistake.
>
> I didn't touch that file at all. I'm not sure what that is. :-P

> Looking at just how similar these 3 implementations are I feel that we
> > could also unify/generalize them into one generic helper implementation
> > and leave the object marshaling up to the type specific methods.
>
> You could put it in Objects/stringlib.
>

I figured. I will when I am able to get to it.

- John

Antoine Pitrou <pitrou@free.fr> added the comment:

> This looks like a dubious micro-optimization. If len == 0,
> all loops will exit early anyway (same for similar snippets in bytesobject.c and unicodeobject.c).

You are right about those lines in particular; 'dubious' as you say. 
Though, the point overall was that the general implementation was noticeably
faster (regardless of those smaller changes). 

However, I do think that the single check for len == 0 saves time particularly
from do_xstrip where we call _getbuffer().


> bytearray objects are mutable, so you can't return the same object without breaking expected semantics. e.g.:

I see. I must have been trigger happy.


> diff -r b5ccdf7c032a Python/bltinmodule.c
> Binary file Python/bltinmodule.c has changed

> Uh, what's this? bltinmodule.c shouldn't be considered a binary file.
> It probably means you added a NUL byte in it by mistake.

I didn't touch that file at all. I'm not sure what that is. :-P

> You could put it in Objects/stringlib.

I figured. I will when I am able to get to it.

Moved {l,r,}strip to stringlib as well as the bloom filters from unicodeobject. I think it cleans things up nicely. Now there is one implementation for all 3 types. This patch also improves performance for bytearray and slightly for bytes. I see no delta for unicodeobject.

Added a test case for the mistake I made before (returning inc ref for mutable type).

grep -R couldn't find any other references to _PyUnicode_XStrip so I removed it. Though, someone with better knowledge of this should confirm.

The patch no longer applies cleanly. Is there enough interest in this to justify rebasing?

> The patch no longer applies cleanly. Is there enough interest in this to justify rebasing?

Yes, I think it's worth it.

New patch.

Please double check the removal of _PyUnicode_XStrip.

> Please double check the removal of _PyUnicode_XStrip.

I think it's ok. It is private, undocumented, and the only references
returned by Google Code Search are in Python itself.

I'll review the patch later, thank you.