Support Tempo on IBM s390x

[pavolloffay]

What this PR does:

Which issue(s) this PR fixes:
Fixes #

Checklist

• Tests updated
• Documentation added
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[pavolloffay]

the e2e test failed. Can someone restart them?

[joe-elliott]

Unfortunately, we are unable to merge this due to major performance regressions. Benchmarks from the vparquet3 package here:

benchstat

goos: darwin
goarch: arm64
pkg: github.com/grafana/tempo/tempodb/encoding/vparquet3
cpu: Apple M3 Pro
                                                    │   main.txt   │              branch.txt              │
                                                    │    sec/op    │    sec/op      vs base               │
BackendBlockTraceQL/spanAttValMatch-11                142.3m ± ∞ ¹    160.5m ± ∞ ¹  +12.79% (p=0.008 n=5)
BackendBlockTraceQL/spanAttValNoMatch-11              6.449m ± ∞ ¹    6.689m ± ∞ ¹   +3.73% (p=0.008 n=5)
BackendBlockTraceQL/spanAttIntrinsicMatch-11          57.57m ± ∞ ¹    69.67m ± ∞ ¹  +21.02% (p=0.008 n=5)
BackendBlockTraceQL/spanAttIntrinsicNoMatch-11        6.449m ± ∞ ¹    6.751m ± ∞ ¹   +4.68% (p=0.008 n=5)
BackendBlockTraceQL/resourceAttValMatch-11            511.8m ± ∞ ¹    519.9m ± ∞ ¹   +1.56% (p=0.008 n=5)
BackendBlockTraceQL/resourceAttValNoMatch-11          6.422m ± ∞ ¹    6.467m ± ∞ ¹   +0.70% (p=0.032 n=5)
BackendBlockTraceQL/resourceAttIntrinsicMatch-11      96.28m ± ∞ ¹   102.28m ± ∞ ¹        ~ (p=0.151 n=5)
BackendBlockTraceQL/resourceAttIntrinsicMatch#01-11   6.451m ± ∞ ¹    6.787m ± ∞ ¹   +5.21% (p=0.008 n=5)
BackendBlockTraceQL/traceOrMatch-11                   304.3m ± ∞ ¹    309.2m ± ∞ ¹   +1.60% (p=0.016 n=5)
BackendBlockTraceQL/traceOrNoMatch-11                 304.9m ± ∞ ¹    308.9m ± ∞ ¹        ~ (p=0.095 n=5)
BackendBlockTraceQL/mixedValNoMatch-11                215.8m ± ∞ ¹    215.8m ± ∞ ¹        ~ (p=0.548 n=5)
BackendBlockTraceQL/mixedValMixedMatchAnd-11          6.585m ± ∞ ¹    6.504m ± ∞ ¹   -1.24% (p=0.008 n=5)
BackendBlockTraceQL/mixedValMixedMatchOr-11           200.9m ± ∞ ¹    212.0m ± ∞ ¹        ~ (p=0.095 n=5)
BackendBlockTraceQL/count-11                          426.7m ± ∞ ¹    431.3m ± ∞ ¹   +1.08% (p=0.008 n=5)
BackendBlockTraceQL/struct-11                         561.6m ± ∞ ¹    564.5m ± ∞ ¹        ~ (p=0.421 n=5)
BackendBlockTraceQL/||-11                             195.1m ± ∞ ¹    201.5m ± ∞ ¹   +3.30% (p=0.008 n=5)
BackendBlockTraceQL/mixed-11                          75.85m ± ∞ ¹    81.51m ± ∞ ¹   +7.47% (p=0.008 n=5)
BackendBlockTraceQL/complex-11                        6.616m ± ∞ ¹    7.607m ± ∞ ¹  +14.97% (p=0.008 n=5)
geomean                                               65.03m          68.20m         +4.88%
¹ need >= 6 samples for confidence interval at level 0.95

                                                    │   main.txt    │              branch.txt              │
                                                    │      B/s      │      B/s       vs base               │
BackendBlockTraceQL/spanAttValMatch-11                213.1Mi ± ∞ ¹   188.9Mi ± ∞ ¹  -11.34% (p=0.008 n=5)
BackendBlockTraceQL/spanAttValNoMatch-11              340.9Mi ± ∞ ¹   328.7Mi ± ∞ ¹   -3.59% (p=0.008 n=5)
BackendBlockTraceQL/spanAttIntrinsicMatch-11          548.5Mi ± ∞ ¹   453.3Mi ± ∞ ¹  -17.37% (p=0.008 n=5)
BackendBlockTraceQL/spanAttIntrinsicNoMatch-11        340.9Mi ± ∞ ¹   325.6Mi ± ∞ ¹   -4.47% (p=0.008 n=5)
BackendBlockTraceQL/resourceAttValMatch-11            58.26Mi ± ∞ ¹   57.36Mi ± ∞ ¹   -1.54% (p=0.008 n=5)
BackendBlockTraceQL/resourceAttValNoMatch-11          188.3Mi ± ∞ ¹   187.0Mi ± ∞ ¹   -0.69% (p=0.032 n=5)
BackendBlockTraceQL/resourceAttIntrinsicMatch-11      310.4Mi ± ∞ ¹   292.2Mi ± ∞ ¹        ~ (p=0.151 n=5)
BackendBlockTraceQL/resourceAttIntrinsicMatch#01-11   198.4Mi ± ∞ ¹   188.6Mi ± ∞ ¹   -4.95% (p=0.008 n=5)
BackendBlockTraceQL/traceOrMatch-11                   8.183Mi ± ∞ ¹   8.049Mi ± ∞ ¹   -1.63% (p=0.016 n=5)
BackendBlockTraceQL/traceOrNoMatch-11                 8.163Mi ± ∞ ¹   8.059Mi ± ∞ ¹        ~ (p=0.095 n=5)
BackendBlockTraceQL/mixedValNoMatch-11                13.55Mi ± ∞ ¹   13.55Mi ± ∞ ¹        ~ (p=0.421 n=5)
BackendBlockTraceQL/mixedValMixedMatchAnd-11          224.2Mi ± ∞ ¹   227.0Mi ± ∞ ¹   +1.25% (p=0.008 n=5)
BackendBlockTraceQL/mixedValMixedMatchOr-11           66.89Mi ± ∞ ¹   63.38Mi ± ∞ ¹        ~ (p=0.063 n=5)
BackendBlockTraceQL/count-11                          69.85Mi ± ∞ ¹   69.10Mi ± ∞ ¹   -1.06% (p=0.008 n=5)
BackendBlockTraceQL/struct-11                         15.08Mi ± ∞ ¹   15.00Mi ± ∞ ¹        ~ (p=0.421 n=5)
BackendBlockTraceQL/||-11                             153.6Mi ± ∞ ¹   148.7Mi ± ∞ ¹   -3.20% (p=0.008 n=5)
BackendBlockTraceQL/mixed-11                          389.3Mi ± ∞ ¹   362.3Mi ± ∞ ¹   -6.95% (p=0.008 n=5)
BackendBlockTraceQL/complex-11                        397.7Mi ± ∞ ¹   351.3Mi ± ∞ ¹  -11.66% (p=0.008 n=5)
geomean                                               106.5Mi         101.7Mi         -4.57%
¹ need >= 6 samples for confidence interval at level 0.95

                                                    │  main.txt   │             branch.txt              │
                                                    │  MB_io/op   │  MB_io/op    vs base                │
BackendBlockTraceQL/spanAttValMatch-11                31.80 ± ∞ ¹   31.80 ± ∞ ¹       ~ (p=1.000 n=5) ²
BackendBlockTraceQL/spanAttValNoMatch-11              2.305 ± ∞ ¹   2.305 ± ∞ ¹       ~ (p=1.000 n=5) ²
BackendBlockTraceQL/spanAttIntrinsicMatch-11          33.11 ± ∞ ¹   33.11 ± ∞ ¹       ~ (p=1.000 n=5) ²
BackendBlockTraceQL/spanAttIntrinsicNoMatch-11        2.305 ± ∞ ¹   2.305 ± ∞ ¹       ~ (p=1.000 n=5) ²
BackendBlockTraceQL/resourceAttValMatch-11            31.27 ± ∞ ¹   31.27 ± ∞ ¹       ~ (p=1.000 n=5) ²
BackendBlockTraceQL/resourceAttValNoMatch-11          1.268 ± ∞ ¹   1.268 ± ∞ ¹       ~ (p=1.000 n=5) ²
BackendBlockTraceQL/resourceAttIntrinsicMatch-11      31.34 ± ∞ ¹   31.34 ± ∞ ¹       ~ (p=1.000 n=5) ²
BackendBlockTraceQL/resourceAttIntrinsicMatch#01-11   1.342 ± ∞ ¹   1.342 ± ∞ ¹       ~ (p=1.000 n=5) ²
BackendBlockTraceQL/traceOrMatch-11                   2.610 ± ∞ ¹   2.610 ± ∞ ¹       ~ (p=1.000 n=5) ²
BackendBlockTraceQL/traceOrNoMatch-11                 2.610 ± ∞ ¹   2.610 ± ∞ ¹       ~ (p=1.000 n=5) ²
BackendBlockTraceQL/mixedValNoMatch-11                3.067 ± ∞ ¹   3.067 ± ∞ ¹       ~ (p=1.000 n=5) ²
BackendBlockTraceQL/mixedValMixedMatchAnd-11          1.548 ± ∞ ¹   1.548 ± ∞ ¹       ~ (p=1.000 n=5) ²
BackendBlockTraceQL/mixedValMixedMatchOr-11           14.09 ± ∞ ¹   14.09 ± ∞ ¹       ~ (p=1.000 n=5) ²
BackendBlockTraceQL/count-11                          31.25 ± ∞ ¹   31.25 ± ∞ ¹       ~ (p=1.000 n=5) ²
BackendBlockTraceQL/struct-11                         8.878 ± ∞ ¹   8.878 ± ∞ ¹       ~ (p=1.000 n=5) ²
BackendBlockTraceQL/||-11                             31.43 ± ∞ ¹   31.43 ± ∞ ¹       ~ (p=1.000 n=5) ²
BackendBlockTraceQL/mixed-11                          30.96 ± ∞ ¹   30.96 ± ∞ ¹       ~ (p=1.000 n=5) ²
BackendBlockTraceQL/complex-11                        2.762 ± ∞ ¹   2.788 ± ∞ ¹       ~ (p=0.548 n=5)
geomean                                               7.264         7.268        +0.05%
¹ need >= 6 samples for confidence interval at level 0.95
² all samples are equal

                                                    │   main.txt    │                branch.txt                │
                                                    │     B/op      │      B/op        vs base                 │
BackendBlockTraceQL/spanAttValMatch-11                99.11Mi ± ∞ ¹    306.76Mi ± ∞ ¹   +209.52% (p=0.008 n=5)
BackendBlockTraceQL/spanAttValNoMatch-11              7.397Mi ± ∞ ¹    10.755Mi ± ∞ ¹    +45.40% (p=0.008 n=5)
BackendBlockTraceQL/spanAttIntrinsicMatch-11          14.95Mi ± ∞ ¹    233.60Mi ± ∞ ¹  +1462.91% (p=0.008 n=5)
BackendBlockTraceQL/spanAttIntrinsicNoMatch-11        7.368Mi ± ∞ ¹    11.261Mi ± ∞ ¹    +52.84% (p=0.008 n=5)
BackendBlockTraceQL/resourceAttValMatch-11            664.8Mi ± ∞ ¹     680.4Mi ± ∞ ¹     +2.35% (p=0.008 n=5)
BackendBlockTraceQL/resourceAttValNoMatch-11          7.303Mi ± ∞ ¹     7.282Mi ± ∞ ¹          ~ (p=0.310 n=5)
BackendBlockTraceQL/resourceAttIntrinsicMatch-11      59.29Mi ± ∞ ¹     98.75Mi ± ∞ ¹    +66.56% (p=0.008 n=5)
BackendBlockTraceQL/resourceAttIntrinsicMatch#01-11   7.401Mi ± ∞ ¹    11.037Mi ± ∞ ¹    +49.12% (p=0.008 n=5)
BackendBlockTraceQL/traceOrMatch-11                   36.17Mi ± ∞ ¹     34.42Mi ± ∞ ¹          ~ (p=0.690 n=5)
BackendBlockTraceQL/traceOrNoMatch-11                 33.53Mi ± ∞ ¹     35.40Mi ± ∞ ¹          ~ (p=0.548 n=5)
BackendBlockTraceQL/mixedValNoMatch-11                7.900Mi ± ∞ ¹    12.588Mi ± ∞ ¹    +59.34% (p=0.008 n=5)
BackendBlockTraceQL/mixedValMixedMatchAnd-11          7.436Mi ± ∞ ¹     7.469Mi ± ∞ ¹          ~ (p=0.421 n=5)
BackendBlockTraceQL/mixedValMixedMatchOr-11           8.588Mi ± ∞ ¹   223.923Mi ± ∞ ¹  +2507.25% (p=0.008 n=5)
BackendBlockTraceQL/count-11                          454.4Mi ± ∞ ¹     467.6Mi ± ∞ ¹     +2.89% (p=0.008 n=5)
BackendBlockTraceQL/struct-11                         22.64Mi ± ∞ ¹     88.31Mi ± ∞ ¹   +290.09% (p=0.008 n=5)
BackendBlockTraceQL/||-11                             13.33Mi ± ∞ ¹     81.32Mi ± ∞ ¹   +510.13% (p=0.008 n=5)
BackendBlockTraceQL/mixed-11                          17.55Mi ± ∞ ¹     83.84Mi ± ∞ ¹   +377.86% (p=0.008 n=5)
BackendBlockTraceQL/complex-11                        7.135Mi ± ∞ ¹    22.077Mi ± ∞ ¹   +209.42% (p=0.008 n=5)
geomean                                               22.26Mi           51.90Mi         +133.13%
¹ need >= 6 samples for confidence interval at level 0.95

                                                    │   main.txt   │              branch.txt               │
                                                    │  allocs/op   │   allocs/op    vs base                │
BackendBlockTraceQL/spanAttValMatch-11                960.5k ± ∞ ¹   1092.0k ± ∞ ¹   +13.69% (p=0.008 n=5)
BackendBlockTraceQL/spanAttValNoMatch-11              101.2k ± ∞ ¹    101.3k ± ∞ ¹    +0.16% (p=0.008 n=5)
BackendBlockTraceQL/spanAttIntrinsicMatch-11          158.0k ± ∞ ¹    287.8k ± ∞ ¹   +82.13% (p=0.008 n=5)
BackendBlockTraceQL/spanAttIntrinsicNoMatch-11        101.1k ± ∞ ¹    101.2k ± ∞ ¹    +0.11% (p=0.008 n=5)
BackendBlockTraceQL/resourceAttValMatch-11            4.840M ± ∞ ¹    4.976M ± ∞ ¹    +2.82% (p=0.008 n=5)
BackendBlockTraceQL/resourceAttValNoMatch-11          101.2k ± ∞ ¹    101.2k ± ∞ ¹         ~ (p=0.119 n=5)
BackendBlockTraceQL/resourceAttIntrinsicMatch-11      628.3k ± ∞ ¹    767.8k ± ∞ ¹   +22.20% (p=0.008 n=5)
BackendBlockTraceQL/resourceAttIntrinsicMatch#01-11   101.2k ± ∞ ¹    101.7k ± ∞ ¹    +0.49% (p=0.008 n=5)
BackendBlockTraceQL/traceOrMatch-11                   292.0k ± ∞ ¹    316.4k ± ∞ ¹    +8.35% (p=0.008 n=5)
BackendBlockTraceQL/traceOrNoMatch-11                 291.3k ± ∞ ¹    316.5k ± ∞ ¹    +8.67% (p=0.008 n=5)
BackendBlockTraceQL/mixedValNoMatch-11                102.0k ± ∞ ¹    114.7k ± ∞ ¹   +12.45% (p=0.008 n=5)
BackendBlockTraceQL/mixedValMixedMatchAnd-11          101.2k ± ∞ ¹    101.2k ± ∞ ¹         ~ (p=0.214 n=5)
BackendBlockTraceQL/mixedValMixedMatchOr-11           106.2k ± ∞ ¹    159.9k ± ∞ ¹   +50.63% (p=0.008 n=5)
BackendBlockTraceQL/count-11                          2.846M ± ∞ ¹    2.982M ± ∞ ¹    +4.80% (p=0.008 n=5)
BackendBlockTraceQL/struct-11                         131.0k ± ∞ ¹    344.0k ± ∞ ¹  +162.54% (p=0.008 n=5)
BackendBlockTraceQL/||-11                             169.0k ± ∞ ¹    311.6k ± ∞ ¹   +84.37% (p=0.008 n=5)
BackendBlockTraceQL/mixed-11                          197.2k ± ∞ ¹    328.0k ± ∞ ¹   +66.37% (p=0.008 n=5)
BackendBlockTraceQL/complex-11                        101.6k ± ∞ ¹    103.1k ± ∞ ¹    +1.47% (p=0.008 n=5)
geomean                                               237.3k          293.1k         +23.52%
¹ need >= 6 samples for confidence interval at level 0.95

We are seeing major increases in allocs in some changed funcs in the s390x merge. These two lines in particular are allocing slices unnecessarily. We can move them into the big endian check.

https://github.com/parquet-go/parquet-go/blob/main/internal/bitpack/unpack_int64_purego.go#L15
https://github.com/parquet-go/parquet-go/blob/main/internal/bitpack/unpack_int32_purego.go#L15

We are also concerned about the check itself at such a low level, but hoping that compiler optimizations make it a no-op.

[joe-elliott]

I'll also note that the performance on s390x will likely be very poor due to these allocs. A better implementation might be to flip the endian-ness in place instead of allocing and writing to a new buffer, but that will not block this change.

[joe-elliott]

Other concerns:

Simple and possibly not meaningful, but this var could go inside the if:
https://github.com/parquet-go/parquet-go/pull/173/files#diff-2ebf23590706aa4f62830662a7f476dd9dd3d230f6daf0d00c4374350b5836a9R156

We are also concerned with the empty interface here. We think this will alloc to wrap the passed value. Can we just not call this func if BigEndian is false?

https://github.com/parquet-go/parquet-go/pull/173/files#diff-2ebf23590706aa4f62830662a7f476dd9dd3d230f6daf0d00c4374350b5836a9R156

Also concerned about this dereference, but that will be removed from our code path if we do the above and not call getOffset:

https://github.com/parquet-go/parquet-go/pull/173/files#diff-c9f2b3e1424786e1b0b7ece6d1d0080954a5f8c097bf410b56581ef2e258c643R998

[pavolloffay]

parquet-go/parquet-go#174

[pavolloffay]

@joe-elliott the PR was merged and updated here

[github-actions]

This PR must be merged before a backport PR will be created.

[github-actions]

The backport to release-v2.6 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new branch
git switch --create backport-4175-to-release-v2.6 origin/release-v2.6
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x e37f481778c778ccf006b7d0e7a5c56fcef108fd

When the conflicts are resolved, stage and commit the changes:

git add . && git cherry-pick --continue

If you have the GitHub CLI installed:

# Push the branch to GitHub:
git push --set-upstream origin backport-4175-to-release-v2.6
# Create the PR body template
PR_BODY=$(gh pr view 4175 --json body --template 'Backport e37f481778c778ccf006b7d0e7a5c56fcef108fd from #4175{{ "\n\n---\n\n" }}{{ index . "body" }}')
# Create the PR on GitHub
echo "${PR_BODY}" | gh pr create --title '[release-v2.6]  Support Tempo on IBM s390x ' --body-file - --label 'type/bug' --label 'backport' --base release-v2.6 --milestone release-v2.6 --web

Or, if you don't have the GitHub CLI installed (we recommend you install it!):

# Push the branch to GitHub:
git push --set-upstream origin backport-4175-to-release-v2.6

# Create a pull request where the `base` branch is `release-v2.6` and the `compare`/`head` branch is `backport-4175-to-release-v2.6`.

# Remove the local backport branch
git switch main
git branch -D backport-4175-to-release-v2.6