Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

all: consider using type-params #868

Open
1 of 7 tasks
sbinet opened this issue Aug 20, 2021 · 6 comments
Open
1 of 7 tasks

all: consider using type-params #868

sbinet opened this issue Aug 20, 2021 · 6 comments

Comments

@sbinet
Copy link
Member

sbinet commented Aug 20, 2021

Type parameters (a.k.a. "generics") may land in Go-1.18.

we might want to consider using those for:

  • groot/rhist.TH1{I,F,D}
  • groot/rcont.Array{C,D,F,I,L,L64,S}
  • groot/rtree ?
  • fmom.P4
  • sliceop/f64s (and use the new slices package from stdlib)
  • fwk.Store
  • hbook.H1D, hbook.H2D, ...
@sbinet
Copy link
Member Author

sbinet commented Aug 23, 2021

for groot/rcont.Array (and groot/rbytes.{R,W}Buffer}) one would need a way to type-switch on the type parameter.
ie: golang/go#45380

sbinet added a commit to sbinet-hep/hep that referenced this issue Apr 4, 2022
Move and generalize code from sliceop/f64s into sliceop.

name                old time/op    new time/op    delta
Take/Len=2-8          2.69ns ± 1%    2.71ns ± 2%   +0.75%  (p=0.043 n=18+18)
Take/Len=4-8          3.18ns ± 1%    3.23ns ± 3%   +1.27%  (p=0.005 n=19+20)
Take/Len=8-8          3.83ns ± 1%    3.73ns ± 1%   -2.69%  (p=0.000 n=20+19)
Take/Len=128-8        44.1ns ± 4%    35.9ns ± 2%  -18.61%  (p=0.000 n=20+20)
Take/Len=1024-8        337ns ± 2%     280ns ± 6%  -16.73%  (p=0.000 n=19+20)
Take/Len=1048576-8    1.37ms ± 3%    1.31ms ± 5%   -3.97%  (p=0.000 n=19+20)

name                old alloc/op   new alloc/op   delta
Take/Len=2-8           0.00B          0.00B          ~     (all equal)
Take/Len=4-8           0.00B          0.00B          ~     (all equal)
Take/Len=8-8           0.00B          0.00B          ~     (all equal)
Take/Len=128-8         0.00B          0.00B          ~     (all equal)
Take/Len=1024-8        0.00B          0.00B          ~     (all equal)
Take/Len=1048576-8     0.00B          0.00B          ~     (all equal)

name                old allocs/op  new allocs/op  delta
Take/Len=2-8            0.00           0.00          ~     (all equal)
Take/Len=4-8            0.00           0.00          ~     (all equal)
Take/Len=8-8            0.00           0.00          ~     (all equal)
Take/Len=128-8          0.00           0.00          ~     (all equal)
Take/Len=1024-8         0.00           0.00          ~     (all equal)
Take/Len=1048576-8      0.00           0.00          ~     (all equal)

Updates go-hep#868.
sbinet added a commit to sbinet-hep/hep that referenced this issue Apr 4, 2022
Move and generalize code from sliceop/f64s into sliceop.

name                old time/op    new time/op    delta
Take/Len=2-8          2.69ns ± 1%    2.71ns ± 2%   +0.75%  (p=0.043 n=18+18)
Take/Len=4-8          3.18ns ± 1%    3.23ns ± 3%   +1.27%  (p=0.005 n=19+20)
Take/Len=8-8          3.83ns ± 1%    3.73ns ± 1%   -2.69%  (p=0.000 n=20+19)
Take/Len=128-8        44.1ns ± 4%    35.9ns ± 2%  -18.61%  (p=0.000 n=20+20)
Take/Len=1024-8        337ns ± 2%     280ns ± 6%  -16.73%  (p=0.000 n=19+20)
Take/Len=1048576-8    1.37ms ± 3%    1.31ms ± 5%   -3.97%  (p=0.000 n=19+20)

name                old alloc/op   new alloc/op   delta
Take/Len=2-8           0.00B          0.00B          ~     (all equal)
Take/Len=4-8           0.00B          0.00B          ~     (all equal)
Take/Len=8-8           0.00B          0.00B          ~     (all equal)
Take/Len=128-8         0.00B          0.00B          ~     (all equal)
Take/Len=1024-8        0.00B          0.00B          ~     (all equal)
Take/Len=1048576-8     0.00B          0.00B          ~     (all equal)

name                old allocs/op  new allocs/op  delta
Take/Len=2-8            0.00           0.00          ~     (all equal)
Take/Len=4-8            0.00           0.00          ~     (all equal)
Take/Len=8-8            0.00           0.00          ~     (all equal)
Take/Len=128-8          0.00           0.00          ~     (all equal)
Take/Len=1024-8         0.00           0.00          ~     (all equal)
Take/Len=1048576-8      0.00           0.00          ~     (all equal)

Updates go-hep#868.
@dolmen
Copy link
Contributor

dolmen commented Apr 28, 2022

Just be aware that Go 1.18 generics are not faster for all types.

See this detailed analysis: https://planetscale.com/blog/generics-can-make-your-go-code-slower

@sbinet
Copy link
Member Author

sbinet commented Apr 28, 2022

yep.

for example, I am not too sure about the rhist.TH1x (where x is a numeric type) as all the code has already been generated and it's a fixed set.
(more or less the same argument for the hbook.H{1,2}x types where x could be intXX (but one would need some more special code to handle weights) or floatXX)

@sbinet
Copy link
Member Author

sbinet commented Apr 28, 2022

fwk.Store might make the most sense (parametrized fwk.Get[T](store, k) and fwk.Put[T](store, k, v) functions, like the the groot/riofs.Get[T] top-level function)

@dolmen
Copy link
Contributor

dolmen commented May 4, 2022

What is the status of the fwk project? Is API stability required? Is it still work in progress?

@sbinet
Copy link
Member Author

sbinet commented May 5, 2022

all the packages in go-hep.org/x/hep inherit the v0.x.y tag from the hep module, so no API stability is required.
of course, we try not to modify everything for the sake of modifying things :)
(and we try to provide go fmt -r rewrite rules or eg scripts to ease the migration when possible, but that isn't a requirement either)

for API changes, it's best to discuss changes on the mailing list before actually doing the work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants