~eliasnaur/gio-patches

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
5 4

[PATCH gio] text: optimize faceCache.hashGIDs

Dominik Honnef <dominik@honnef.co>
Details
Message ID
<20220628150919.96911-1-dominik@honnef.co>
DKIM signature
missing
Download raw message
Patch: +3 -1
Use binary.LittleEndian directly instead of going through the
binary.Write indirection. This allows the following optimizations to
occur:

  - We can reuse our own byte slice between iterations
  - We don't have to put g.ID in an interface value
  - h doesn't escape
  - PutUint32 gets inlined

On top of that, the argument to maphash.Hash.Write doesn't escape, so b
doesn't move to the heap.

Signed-off-by: Dominik Honnef <dominik@honnef.co>
---
 text/shaper.go | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/text/shaper.go b/text/shaper.go
index 338d215e..942c8097 100644
--- a/text/shaper.go
+++ b/text/shaper.go
@@ -156,8 +156,10 @@ func (f *faceCache) hashGIDs(layout Layout) uint64 {
	}
	var h maphash.Hash
	h.SetSeed(f.seed)
	var b [4]byte
	for _, g := range layout.Glyphs {
		binary.Write(&h, binary.LittleEndian, g.ID)
		binary.LittleEndian.PutUint32(b[:], uint32(g.ID))
		h.Write(b[:])
	}
	return h.Sum64()
}
-- 
2.36.1
Details
Message ID
<CL1V6FJIWNL1.2YCBN5TUDJS33@clrinfopc42>
In-Reply-To
<20220628150919.96911-1-dominik@honnef.co> (view parent)
DKIM signature
pass
Download raw message
On Tue Jun 28, 2022 at 17:09 CET, Dominik Honnef wrote:
> Use binary.LittleEndian directly instead of going through the
> binary.Write indirection. This allows the following optimizations to
> occur:
>
> - We can reuse our own byte slice between iterations
> - We don't have to put g.ID in an interface value
> - h doesn't escape
> - PutUint32 gets inlined
>
> On top of that, the argument to maphash.Hash.Write doesn't escape, so b
> doesn't move to the heap.
>
> Signed-off-by: Dominik Honnef <dominik@honnef.co>
> ---
> text/shaper.go | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/text/shaper.go b/text/shaper.go
> index 338d215e..942c8097 100644
> --- a/text/shaper.go
> +++ b/text/shaper.go
> @@ -156,8 +156,10 @@ func (f *faceCache) hashGIDs(layout Layout) uint64
> {
> }
> var h maphash.Hash
> h.SetSeed(f.seed)
> + var b [4]byte

b := make([]byte, 4)

instead?

> for _, g := range layout.Glyphs {
> - binary.Write(&h, binary.LittleEndian, g.ID)
> + binary.LittleEndian.PutUint32(b[:], uint32(g.ID))

and do:
 binary.LittleEndian.PutUint32(b, uint32(g.ID))
instead ?

(it used to be that slicing an array would incur a performance hit.)

> + h.Write(b[:])
> }
> return h.Sum64()
> }
> --
> 2.36.1

-s

[gio/patches] build failed

builds.sr.ht <builds@sr.ht>
Details
Message ID
<CL1VHAJTRJBD.125JUDC45D03X@cirno>
In-Reply-To
<20220628150919.96911-1-dominik@honnef.co> (view parent)
DKIM signature
missing
Download raw message
gio/patches: FAILED in 19m26s

[text: optimize faceCache.hashGIDs][0] from [Dominik Honnef][1]

[0]: https://lists.sr.ht/~eliasnaur/gio-patches/patches/33352
[1]: dominik@honnef.co

✗ #789794 FAILED  gio/patches/linux.yml   https://builds.sr.ht/~eliasnaur/job/789794
✓ #789793 SUCCESS gio/patches/freebsd.yml https://builds.sr.ht/~eliasnaur/job/789793
✓ #789795 SUCCESS gio/patches/openbsd.yml https://builds.sr.ht/~eliasnaur/job/789795
✓ #789792 SUCCESS gio/patches/apple.yml   https://builds.sr.ht/~eliasnaur/job/789792
Dominik Honnef <dominik@honnef.co>
Details
Message ID
<87o7ycdbbl.fsf@honnef.co>
In-Reply-To
<CL1V6FJIWNL1.2YCBN5TUDJS33@clrinfopc42> (view parent)
DKIM signature
missing
Download raw message
"Sebastien Binet" <s@sbinet.org> writes:

> On Tue Jun 28, 2022 at 17:09 CET, Dominik Honnef wrote:
>> --- a/text/shaper.go
>> +++ b/text/shaper.go
>> @@ -156,8 +156,10 @@ func (f *faceCache) hashGIDs(layout Layout) uint64
>> {
>> }
>> var h maphash.Hash
>> h.SetSeed(f.seed)
>> + var b [4]byte
>
> b := make([]byte, 4)
>
> instead?
>
>> for _, g := range layout.Glyphs {
>> - binary.Write(&h, binary.LittleEndian, g.ID)
>> + binary.LittleEndian.PutUint32(b[:], uint32(g.ID))
>
> and do:
>  binary.LittleEndian.PutUint32(b, uint32(g.ID))
> instead ?
>
> (it used to be that slicing an array would incur a performance hit.)
>

According to -gcflags=-S, the two produce identical code. Personally I
prefer my version because it signals to readers that we expect the
allocation to stay on the stack. Ultimately I don't care though and can
change it.
Details
Message ID
<CL1WJXWAKQN4.1YP4Y35JZNGMC@clrinfopc42>
In-Reply-To
<87o7ycdbbl.fsf@honnef.co> (view parent)
DKIM signature
pass
Download raw message
On Tue Jun 28, 2022 at 18:03 CET, Dominik Honnef wrote:
> "Sebastien Binet" <s@sbinet.org> writes:
>
> > On Tue Jun 28, 2022 at 17:09 CET, Dominik Honnef wrote:
> >> --- a/text/shaper.go
> >> +++ b/text/shaper.go
> >> @@ -156,8 +156,10 @@ func (f *faceCache) hashGIDs(layout Layout) uint64
> >> {
> >> }
> >> var h maphash.Hash
> >> h.SetSeed(f.seed)
> >> + var b [4]byte
> >
> > b := make([]byte, 4)
> >
> > instead?
> >
> >> for _, g := range layout.Glyphs {
> >> - binary.Write(&h, binary.LittleEndian, g.ID)
> >> + binary.LittleEndian.PutUint32(b[:], uint32(g.ID))
> >
> > and do:
> >  binary.LittleEndian.PutUint32(b, uint32(g.ID))
> > instead ?
> >
> > (it used to be that slicing an array would incur a performance hit.)
> >
>
> According to -gcflags=-S, the two produce identical code. Personally I
> prefer my version because it signals to readers that we expect the
> allocation to stay on the stack. Ultimately I don't care though and can
> change it.

if the 2 produce identical code, then the array version is better.
compilers actually do improve :)

-s
Details
Message ID
<CAMAFT9WAvPt730ZueT2_mLrz22OdL-5M0jRiuG-2t7WLjBzfjw@mail.gmail.com>
In-Reply-To
<20220628150919.96911-1-dominik@honnef.co> (view parent)
DKIM signature
pass
Download raw message
Thanks, merged.

Elias
Reply to thread Export thread (mbox)