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

Jump to bottom

runtime: performance regression due to bad instruction used in morestack_noctxt for ppc64 in CL 425396 [1.19 backport] #57812

Closed
gopherbot opened this issue Jan 16, 2023 · 3 comments
Closed

runtime: performance regression due to bad instruction used in morestack_noctxt for ppc64 in CL 425396 [1.19 backport] #57812

gopherbot opened this issue Jan 16, 2023 · 3 comments
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime.
Milestone
Go1.19.6

Comments

@gopherbot
Copy link

@archanaravindar requested issue #57741 to be considered for backport to the next 1.19 minor release.

@gopherbot please backport this to previous releases Go1.19 and Go1.18
Since the fix 54332 that causes this regression on ppc64 has been likewise back ported to these two releases
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Jan 16, 2023
@gopherbot gopherbot mentioned this issue Jan 16, 2023
Closed
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jan 16, 2023
@gopherbot gopherbot added this to the Go1.19.6 milestone Jan 16, 2023
@archanaravindar
Copy link
Contributor

Since the fix removes the regression in Go1.19 for the following benchmarks, we request this issue to be back ported

StackCopyPtr-64 138ms           85ms%   -38.36% 
StackCopy-64    137ms           83ms%   -39.48% 
StackCopyNoCache-64     15.1ms  2.5ms   -83.49% 
StackCopyWithStkobj-64  39.9ms  14.4ms  -63.89% 
                                                                        
Hash65536-64    23.5µs          3.9µs   -83.28% 
MegEqMap-64     146µs           22µs    -84.63% 
RepeatedLookupStrMapKey1M-64    403µs   63µs    -84.45% 
MakeChan/Struct/40-64   197ns   146ns   -25.83% 
GoMapClear/NonReflexive/10000-64        116µs   18µs    -84.79% 
MapPop10000-64  9.44ms          4.09ms  -56.72%

@gopherbot
Copy link
Author

Change https://go.dev/cl/462335 mentions this issue: [release-branch.go1.19] runtime: fix performance regression in morestack_noctxt on ppc64

@cherrymui cherrymui added the CherryPickApproved Used during the release process for point releases label Jan 18, 2023
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Jan 18, 2023
@gopherbot
Copy link
Author

Closed by merging 73e1aff to release-branch.go1.19.

gopherbot pushed a commit that referenced this issue Jan 18, 2023
@archanaravindar @cherrymui
[release-branch.go1.19] runtime: fix performance regression in morest…
73e1aff 
…ack_noctxt on ppc64

In the fix for 54332 the MOVD R1, R1 instruction was added to
morestack_noctxt function to set the SPWRITE bit. However, the
instruction MOVD R1, R1 results in or r1,r1,r1 which is a special
instruction on ppc64 architecture as it changes the thread priority
and can negatively impact performance in some cases.
More details on such similar nops can be found in Power ISA v3.1
Book II on Power ISA Virtual Environment architecture in the chapter
on Program Priority Registers and Or instructions.
Replacing this by OR R0, R1 has the same affect on setting SPWRITE as
needed by the first fix but does not affect thread priority and
hence does not cause the degradation in performance

Hash65536-64           2.81GB/s ±10%  16.69GB/s ± 0%  +494.44%
Fixes #57812

Change-Id: Ib912e3716c6afd277994d6c1c5b2891f82225d50
Reviewed-on: https://go-review.googlesource.com/c/go/+/461597
Reviewed-by: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Lynn Boger <laboger@linux.vnet.ibm.com>
Auto-Submit: Benny Siegert <bsiegert@gmail.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com>
(cherry picked from commit 1c65b69)
Reviewed-on: https://go-review.googlesource.com/c/go/+/462335
Run-TryBot: Archana Ravindar <aravind5@in.ibm.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime.
Projects
None yet
Milestone
Go1.19.6
Development

No branches or pull requests
3 participants
@gopherbot @archanaravindar @cherrymui