[dfvfs] Changed text file to use split instead of partition

[dfvfs] Changed text file to use split instead of partition

[Joachim Metz, 2017-10-20 18:45:40]

Code updated.

[onager, 2017-10-20 18:58:35]

https://codereview.appspot.com/331050043/diff/20001/dfvfs/helpers/text_file.py
File dfvfs/helpers/text_file.py (right):

https://codereview.appspot.com/331050043/diff/20001/dfvfs/helpers/text_file.p...
dfvfs/helpers/text_file.py:145: non-negative, it is a maximum byte count
(including the trailing
Please fix the indentation here

https://codereview.appspot.com/331050043/diff/20001/dfvfs/helpers/text_file.p...
dfvfs/helpers/text_file.py:180: self._lines_buffer = b''.join([lines_data,
self._lines_buffer])
This is still going to lead to excessive memory usage, as all the splitlines are
copied again into the buffer on each call to readline(). 
Take a look at this code:
https://github.com/log2timeline/plaso/blob/e123f94cb29042737facd31eae8e84933e...

Lines need to be buffered separately to avoid excessive memory usage, and a read
triggered if the line doesn't end with a separator. Splitting and joining the
buffer on ever read triggers a copy of the whole buffer.

https://codereview.appspot.com/331050043/diff/20001/dfvfs/helpers/text_file.p...
dfvfs/helpers/text_file.py:216: of reading up to EOF, whole lines totalling
sizehint bytes are read.
Please fix indentation

[Joachim Metz, 2017-10-21 04:53:28]

Code updated.

[Joachim Metz, 2017-10-21 04:53:58]

https://codereview.appspot.com/331050043/diff/20001/dfvfs/helpers/text_file.py
File dfvfs/helpers/text_file.py (right):

https://codereview.appspot.com/331050043/diff/20001/dfvfs/helpers/text_file.p...
dfvfs/helpers/text_file.py:145: non-negative, it is a maximum byte count
(including the trailing
On 2017/10/20 18:58:35, onager wrote:
> Please fix the indentation here

Done.

https://codereview.appspot.com/331050043/diff/20001/dfvfs/helpers/text_file.p...
dfvfs/helpers/text_file.py:180: self._lines_buffer = b''.join([lines_data,
self._lines_buffer])
On 2017/10/20 18:58:35, onager wrote:
> This is still going to lead to excessive memory usage, as all the splitlines
are
> copied again into the buffer on each call to readline(). 
> Take a look at this code:
>
https://github.com/log2timeline/plaso/blob/e123f94cb29042737facd31eae8e84933e...
> 
> Lines need to be buffered separately to avoid excessive memory usage, and a
read
> triggered if the line doesn't end with a separator. Splitting and joining the
> buffer on ever read triggers a copy of the whole buffer.

Done.

https://codereview.appspot.com/331050043/diff/20001/dfvfs/helpers/text_file.p...
dfvfs/helpers/text_file.py:216: of reading up to EOF, whole lines totalling
sizehint bytes are read.
On 2017/10/20 18:58:35, onager wrote:
> Please fix indentation

Done.

[Joachim Metz, 2017-10-21 04:56:20]

Code updated.

[Joachim Metz, 2017-10-21 04:58:50]

Code updated.

[Joachim Metz, 2017-10-21 05:00:11]

https://codereview.appspot.com/331050043/diff/80001/dfvfs/helpers/text_file.py
File dfvfs/helpers/text_file.py (right):

https://codereview.appspot.com/331050043/diff/80001/dfvfs/helpers/text_file.p...
dfvfs/helpers/text_file.py:115: if (self._lines_buffer and
TODO test scenario where size < len(line)

[Joachim Metz, 2017-10-21 05:11:57]

Code updated.

[Joachim Metz, 2017-10-21 05:14:40]

PTAL

[onager, 2017-10-21 16:08:46]

LGTM

https://codereview.appspot.com/331050043/diff/100001/dfvfs/helpers/text_file.py
File dfvfs/helpers/text_file.py (right):

https://codereview.appspot.com/331050043/diff/100001/dfvfs/helpers/text_file....
dfvfs/helpers/text_file.py:110: if read_buffer[self._end_of_line_length:] !=
self._end_of_line:
Please add a comment here about checking for a partial line, I spent a while
staring at this code trying to work out if there was a bug here.

[Joachim Metz, 2017-10-21 16:22:14]

https://codereview.appspot.com/331050043/diff/100001/dfvfs/helpers/text_file.py
File dfvfs/helpers/text_file.py (right):

https://codereview.appspot.com/331050043/diff/100001/dfvfs/helpers/text_file....
dfvfs/helpers/text_file.py:110: if read_buffer[self._end_of_line_length:] !=
self._end_of_line:
On 2017/10/21 16:08:46, onager wrote:
> Please add a comment here about checking for a partial line, I spent a while
> staring at this code trying to work out if there was a bug here.

However thinking of this approach I think it does not correctly handles
readline() followed by readline(size < len(line))

[Joachim Metz, 2017-10-21 16:36:42]

Code updated.

[Joachim Metz, 2017-10-21 16:39:19]

PTAL

[onager, 2017-10-21 23:55:01]

LGTM

[Joachim Metz, 2017-10-22 04:44:23]

Changes have been merged with master branch. To close the review and clean up
the feature branch you can run: python ./utils/review.py close text_file