[2/2] gnu: icecat: Use libjpeg-turbo instead of bundled libjpeg.

  * gnu/packages/gnuzilla.scm (icecat)[source]: Remove bundled libjpeg.
[inputs]: Add libjpeg-turbo.
[arguments]: Use it.
---
 gnu/packages/gnuzilla.scm | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)
  

On Sun, Nov 27, 2016 at 09:17:07PM +0100, Tobias Geerinckx-Rice wrote:
> * gnu/packages/gnuzilla.scm (icecat)[source]: Remove bundled libjpeg.
> [inputs]: Add libjpeg-turbo.
> [arguments]: Use it.

> -                           ;; Fails with "libjpeg-turbo JCS_EXTENSIONS
> -                           ;; required".
> -                           ;; According to
> -                           ;; http://sourceforge.net/projects/libjpeg-turbo/ ,
> -                           ;; "libjpeg-turbo is a derivative of libjpeg that
> -                           ;; uses MMX, SSE, SSE2, and NEON SIMD instructions
> -                           ;; to accelerate baseline JPEG compression/
> -                           ;; decompression", so we had better not use it
> -                           ;; "--with-system-jpeg"

Will this work on nachines that don't support SSE? My understanding is
that we don't wish to require it.
  

Leo,

On 27/11/16 21:56, Leo Famulari wrote:
> Will this work on nachines that don't support SSE? My understanding is
> that we don't wish to require it.

Why wouldn't it?

I don't often use Icecat, but when I do... I don't see why it would
require SSE by default. ;-) And it's not like we ever patched it to use
IJG's libjpeg to begin with.

Kind regards,

T G-R
  

On Sun, Nov 27, 2016 at 10:02:35PM +0100, Tobias Geerinckx-Rice wrote:
> Leo,
> 
> On 27/11/16 21:56, Leo Famulari wrote:
> > Will this work on nachines that don't support SSE? My understanding is
> > that we don't wish to require it.
> 
> Why wouldn't it?
> 
> I don't often use Icecat, but when I do... I don't see why it would
> require SSE by default. ;-) And it's not like we ever patched it to use
> IJG's libjpeg to begin with.

I'm not sure, but the comment seems to indicate that we don't want to
use it:

;; According to
;; http://sourceforge.net/projects/libjpeg-turbo/ ,
;; "libjpeg-turbo is a derivative of libjpeg that
;; uses MMX, SSE, SSE2, and NEON SIMD instructions
;; to accelerate baseline JPEG compression/
;; decompression", so we had better not use it

But I'm not sure how to interpret that, so I've copied Andreas on my
reply (he wrote that comment).
  

Leo Famulari <leo@famulari.name> writes:

> On Sun, Nov 27, 2016 at 10:02:35PM +0100, Tobias Geerinckx-Rice wrote:
>> Leo,
>> 
>> On 27/11/16 21:56, Leo Famulari wrote:
>> > Will this work on nachines that don't support SSE? My understanding is
>> > that we don't wish to require it.
>> 
>> Why wouldn't it?
>> 
>> I don't often use Icecat, but when I do... I don't see why it would
>> require SSE by default. ;-) And it's not like we ever patched it to use
>> IJG's libjpeg to begin with.
>
> I'm not sure, but the comment seems to indicate that we don't want to
> use it:
>
> ;; According to
> ;; http://sourceforge.net/projects/libjpeg-turbo/ ,
> ;; "libjpeg-turbo is a derivative of libjpeg that
> ;; uses MMX, SSE, SSE2, and NEON SIMD instructions
> ;; to accelerate baseline JPEG compression/
> ;; decompression", so we had better not use it

SSE is pentium III, and SSE2 was introduced in Pentium IV. MMX is even
older. Are we really committed to supporting 15+ year old hardware?

The NEON SIMD instruction seems to be ARM-specific and is available at
least on Cortex A8, which is an ARMv7 design.
  

Marius Bakke <mbakke@fastmail.com> writes:

> Leo Famulari <leo@famulari.name> writes:
>
>> On Sun, Nov 27, 2016 at 10:02:35PM +0100, Tobias Geerinckx-Rice wrote:
>>> Leo,
>>> 
>>> On 27/11/16 21:56, Leo Famulari wrote:
>>> > Will this work on nachines that don't support SSE? My understanding is
>>> > that we don't wish to require it.
>>> 
>>> Why wouldn't it?
>>> 
>>> I don't often use Icecat, but when I do... I don't see why it would
>>> require SSE by default. ;-) And it's not like we ever patched it to use
>>> IJG's libjpeg to begin with.
>>
>> I'm not sure, but the comment seems to indicate that we don't want to
>> use it:
>>
>> ;; According to
>> ;; http://sourceforge.net/projects/libjpeg-turbo/ ,
>> ;; "libjpeg-turbo is a derivative of libjpeg that
>> ;; uses MMX, SSE, SSE2, and NEON SIMD instructions
>> ;; to accelerate baseline JPEG compression/
>> ;; decompression", so we had better not use it
>
> SSE is pentium III, and SSE2 was introduced in Pentium IV. MMX is even
> older. Are we really committed to supporting 15+ year old hardware?

Digging a bit further, SSE2 is actually part of the AMD64
specification[0]. Perhaps we could make it conditional on architecture?

0: https://en.wikipedia.org/wiki/SSE2#CPU_support
  

On Sun, Nov 27, 2016 at 10:50:35PM +0100, Marius Bakke wrote:
> Leo Famulari <leo@famulari.name> writes:
> > I'm not sure, but the comment seems to indicate that we don't want to
> > use it:
> >
> > ;; According to
> > ;; http://sourceforge.net/projects/libjpeg-turbo/ ,
> > ;; "libjpeg-turbo is a derivative of libjpeg that
> > ;; uses MMX, SSE, SSE2, and NEON SIMD instructions
> > ;; to accelerate baseline JPEG compression/
> > ;; decompression", so we had better not use it
> 
> SSE is pentium III, and SSE2 was introduced in Pentium IV. MMX is even
> older. Are we really committed to supporting 15+ year old hardware?

See this earlier thread:

http://lists.gnu.org/archive/html/guix-devel/2016-07/msg01534.html

I think it's fine to use libjpeg-turbo if it can run on those older
processors, or if we decide to drop support for them.

> The NEON SIMD instruction seems to be ARM-specific and is available at
> least on Cortex A8, which is an ARMv7 design.

The extremely popular Cortex A7 [0] also has NEON SIMD.

[0] Allwinner A20
  

On Sun, Nov 27, 2016 at 11:28:45PM +0100, Marius Bakke wrote:
> Marius Bakke <mbakke@fastmail.com> writes:
> 
> > Leo Famulari <leo@famulari.name> writes:
> >
> >> On Sun, Nov 27, 2016 at 10:02:35PM +0100, Tobias Geerinckx-Rice wrote:
> >>> Leo,
> >>> 
> >>> On 27/11/16 21:56, Leo Famulari wrote:
> >>> > Will this work on nachines that don't support SSE? My understanding is
> >>> > that we don't wish to require it.
> >>> 
> >>> Why wouldn't it?
> >>> 
> >>> I don't often use Icecat, but when I do... I don't see why it would
> >>> require SSE by default. ;-) And it's not like we ever patched it to use
> >>> IJG's libjpeg to begin with.
> >>
> >> I'm not sure, but the comment seems to indicate that we don't want to
> >> use it:
> >>
> >> ;; According to
> >> ;; http://sourceforge.net/projects/libjpeg-turbo/ ,
> >> ;; "libjpeg-turbo is a derivative of libjpeg that
> >> ;; uses MMX, SSE, SSE2, and NEON SIMD instructions
> >> ;; to accelerate baseline JPEG compression/
> >> ;; decompression", so we had better not use it
> >
> > SSE is pentium III, and SSE2 was introduced in Pentium IV. MMX is even
> > older. Are we really committed to supporting 15+ year old hardware?
> 
> Digging a bit further, SSE2 is actually part of the AMD64
> specification[0]. Perhaps we could make it conditional on architecture?
> 
> 0: https://en.wikipedia.org/wiki/SSE2#CPU_support

Sure, if it requires SSE2 to run, and we keep the status quo regarding
i686, I don't see why not.
  

Andreas, Leo,

Thanks for the feedback!

On 27/11/16 22:41, Leo Famulari wrote:
> I'm not sure, but the comment seems to indicate that we don't want to
> use it:
> 
> ;; According to
> ;; http://sourceforge.net/projects/libjpeg-turbo/ ,
> ;; "libjpeg-turbo is a derivative of libjpeg that
> ;; uses MMX, SSE, SSE2, and NEON SIMD instructions
> ;; to accelerate baseline JPEG compression/
> ;; decompression", so we had better not use it

There seems to be some misunderstanding here. Sorry if it wasn't clear:
this (libjpeg-turbo) was *already* bundled and used by icecat, as the
‘media/libjpeg’ (sic) that this patch removes. You'll find the same SIMD
code there. For the quote above is missing:

> ;; Fails with "libjpeg-turbo JCS_EXTENSIONS required".

Translation: icecat wouldn't build with Guix's ‘libjpeg’ even if we'd
want that.

And we don't. This isn't some dodgy go-faster fork of IJG's ‘real’
libjpeg. It's intended as a sane alternative to the IJG library, albeit
one that doesn't implement every recent extension. It detects CPU
features at runtime — although they're the status quo now, as Andreas
notes — and falls back to other algorithms that might still be faster.

Dang it. I thought I wrote good (description)s. :-(

Kind regards,

T G-R-turbo
  

On 28/11/16 00:15, Tobias Geerinckx-Rice wrote:
> this (libjpeg-turbo) was *already* bundled and used by icecat, [...]

Some fortuitous grepping revealed that our spice packages also #warns
about *not* being built with libjpeg-turbo. Absent any known drawbacks,
I'd follow upstreams' recommendations in these matters.

Kind regards,

T G-R
  

On Mon, Nov 28, 2016 at 12:15:01AM +0100, Tobias Geerinckx-Rice wrote:
> There seems to be some misunderstanding here. Sorry if it wasn't clear:
> this (libjpeg-turbo) was *already* bundled and used by icecat, as the
> ‘media/libjpeg’ (sic) that this patch removes. You'll find the same SIMD
> code there. For the quote above is missing:

Ah, thanks for the clarification.

> > ;; Fails with "libjpeg-turbo JCS_EXTENSIONS required".
> 
> Translation: icecat wouldn't build with Guix's ‘libjpeg’ even if we'd
> want that.
> 
> And we don't. This isn't some dodgy go-faster fork of IJG's ‘real’
> libjpeg. It's intended as a sane alternative to the IJG library, albeit
> one that doesn't implement every recent extension. It detects CPU
> features at runtime — although they're the status quo now, as Andreas
> notes — and falls back to other algorithms that might still be faster.
> 
> Dang it. I thought I wrote good (description)s. :-(

The description is excellent. Thanks for putting up with my
misunderstanding :)
  

On Mon, Nov 28, 2016 at 12:20:53AM +0100, Tobias Geerinckx-Rice wrote:
> On 28/11/16 00:15, Tobias Geerinckx-Rice wrote:
> > this (libjpeg-turbo) was *already* bundled and used by icecat, [...]
> 
> Some fortuitous grepping revealed that our spice packages also #warns
> about *not* being built with libjpeg-turbo. Absent any known drawbacks,
> I'd follow upstreams' recommendations in these matters.

Now that we have the package, I think we should make spice use it.