← Back to team overview

apport-hackers team mailing list archive

Re: [Merge] lp:~brian-murray/apport/zgrep-fallback into lp:apport

 

I can't say that I like having two code paths for the same thing, but I accept that it's necessary. (Still, if these machines are that tight on RAM, can we increase that a bit?)

I would like this to be a bit faster and simpler though, see inline comment. Thanks!

Diff comments:

> === modified file 'backends/packaging-apt-dpkg.py'
> --- backends/packaging-apt-dpkg.py	2016-08-13 07:09:38 +0000
> +++ backends/packaging-apt-dpkg.py	2016-11-07 18:30:14 +0000
> @@ -1221,9 +1222,25 @@
>  
>              # zgrep is magnitudes faster than a 'gzip.open/split() loop'
>              package = None
> -            zgrep = subprocess.Popen(['zgrep', '-m1', '^%s[[:space:]]' % file, map],
> -                                     stdout=subprocess.PIPE, stderr=subprocess.PIPE)
> -            out = zgrep.communicate()[0].decode('UTF-8')
> +            try:
> +                zgrep = subprocess.Popen(['zgrep', '-m1', '^%s[[:space:]]' % file, map],
> +                                         stdout=subprocess.PIPE, stderr=subprocess.PIPE)
> +                out = zgrep.communicate()[0].decode('UTF-8')
> +            except OSError as e:
> +                if e.errno != errno.ENOMEM:
> +                    raise
> +                import gzip
> +                with gzip.open('%s' % map, 'rb') as contents:
> +                    out = ''
> +                    for line in contents:
> +                        try:
> +                            line = line.decode('UTF-8').rstrip('\n')
> +                        # 2016-11-01 this should be better
> +                        except UnicodeDecodeError:
> +                            continue

The decode() makes things slower, and this exception handling is ugly -- I'd rather recommend to do "file_b = file.encode()" and do the startswith() test for file_b. This works fine on bytes, doesn't need decoding, and is simpler and faster.

> +                        if line.startswith(file):
> +                            out = line
> +                            break
>              # we do not check the return code, since zgrep -m1 often errors out
>              # with 'stdout: broken pipe'
>              if out:


-- 
https://code.launchpad.net/~brian-murray/apport/zgrep-fallback/+merge/310218
Your team Apport upstream developers is requested to review the proposed merge of lp:~brian-murray/apport/zgrep-fallback into lp:apport.


References