Skip to content

all: use unsafe.{Slice,SliceData,String,StringData} to simplify the code #54854

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.

Sign up for GitHub

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
Closed
cuiweixie opened this issue Sep 3, 2022 · 16 comments
Closed

all: use unsafe.{Slice,SliceData,String,StringData} to simplify the code #54854

cuiweixie opened this issue Sep 3, 2022 · 16 comments
Assignees
@cuiweixie
Labels
NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@cuiweixie
Copy link
Contributor

since unsafe.{Slice,SliceData,String,StringData} has been added, some code that uses reflect.{SliceHeader, StringHeader} could be simplify.
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/428156 mentions this issue: internal/fuzz: simplify code using unsafe.{SliceData,Slice}

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/428154 mentions this issue: crypto/x509/internal/macos: simplify code using unsafe.SliceData at BytesToCFData

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/428157 mentions this issue: runtime: simplify code using unsafe.Sloce at TestMemmoveOverflow

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/428155 mentions this issue: bytes: simplify code using unsafe.SliceData

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/428158 mentions this issue: strings: simplify code using unsafe.StringData

@cuiweixie
Copy link
Contributor Author

cuiweixie commented Sep 3, 2022 •
edited
Loading

these cls are some demos.

@hopehook hopehook added this to the Backlog milestone Sep 3, 2022
@hopehook hopehook added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 3, 2022
@hopehook
Copy link
Member

hopehook commented Sep 3, 2022

CC @ianlancetaylor @mdempsky @randall77

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/428175 mentions this issue: cmd/cgo: simplify code using unsafe.Slice

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/428176 mentions this issue: syscall: simplify code using unsafe.Slice

gopherbot pushed a commit that referenced this issue Sep 7, 2022
@cuiweixie @gopherbot
crypto/x509/internal/macos: simplify code using unsafe.{SliceData,Str…
db9e549 
…ingData}

Updates #54854

Change-Id: I8a64a1176cbe16489e1fd21c66a7abc7d8b8e9b3
Reviewed-on: https://go-review.googlesource.com/c/go/+/428154
Run-TryBot: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Ian Lance Taylor <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
gopherbot pushed a commit that referenced this issue Sep 7, 2022
@cuiweixie @gopherbot
bytes: simplify code using unsafe.SliceData
2a627af 
Updates #54854

Change-Id: I9c14f9fa595f73eae44eb714abc5d486915893c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/428155
Reviewed-by: Ian Lance Taylor <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Ian Lance Taylor <[email protected]>
gopherbot pushed a commit that referenced this issue Sep 7, 2022
@cuiweixie @gopherbot
internal/fuzz: simplify code using unsafe.{SliceData,Slice}
92b8f4e 
Updates #54854

Change-Id: If70b11c0fc920b7214fcff249da9e3c4ac1de991
Reviewed-on: https://go-review.googlesource.com/c/go/+/428156
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
Run-TryBot: Bryan Mills <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
gopherbot pushed a commit that referenced this issue Sep 7, 2022
@cuiweixie @gopherbot
strings: simplify code using unsafe.StringData
2ee075d 
Updates #54854

Change-Id: I93396dc92bd2decba895f2d059e1aeffcd22312c
Reviewed-on: https://go-review.googlesource.com/c/go/+/428158
Run-TryBot: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Auto-Submit: Bryan Mills <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>
@hopehook hopehook added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Sep 7, 2022
gopherbot pushed a commit that referenced this issue Sep 7, 2022
@cuiweixie @gopherbot
runtime: simplify code using unsafe.{Slice,String}
4fe4601 
Updates #54854

Change-Id: Ie18665e93e477b6f220acf4c6c070b2af4343064
Reviewed-on: https://go-review.googlesource.com/c/go/+/428157
Run-TryBot: Ian Lance Taylor <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
Auto-Submit: Ian Lance Taylor <[email protected]>
gopherbot pushed a commit that referenced this issue Sep 7, 2022
@cuiweixie @gopherbot
syscall: simplify code using unsafe.Slice
ba1ef54 
Updates #54854

Change-Id: Ibaf4eea14a6259cdbca79e9e95db1602966f18e6
Reviewed-on: https://go-review.googlesource.com/c/go/+/428176
Reviewed-by: Bryan Mills <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
Auto-Submit: Ian Lance Taylor <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
@cuishuang cuishuang mentioned this issue Sep 7, 2022
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/428975 mentions this issue: test: simplify code using unsafe.SliceData

@cuishuang cuishuang mentioned this issue Sep 7, 2022
Closed
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/428996 mentions this issue: runtime: simplify code using unsafe.{Slice,String}

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/429115 mentions this issue: all: use unsafe.{Slice, SliceData, String, StringData} to simplify code

@hopehook hopehook changed the title all: using unsafe.{Slice,SliceData,String,StringData} to simplify code all: use unsafe.{Slice,SliceData,String,StringData} to simplify the code Sep 8, 2022
@julieqiu julieqiu removed this from Go Security Sep 8, 2022
gopherbot pushed a commit that referenced this issue Sep 8, 2022
@hopehook @gopherbot
all: use unsafe.{Slice, SliceData, String, StringData} to simplify code
a9a3982 
Because most of these APIs are recently supported, we can only do some
advancement work as much as possible under the premise of compatibility.

For #54854.

Change-Id: Id15d11288bf23902570d54eaf2704a5264210b2e
Reviewed-on: https://go-review.googlesource.com/c/go/+/429115
Reviewed-by: Ian Lance Taylor <[email protected]>
Run-TryBot: hopehook <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
Auto-Submit: Ian Lance Taylor <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/429756 mentions this issue: runtime: use unsafe.{Slice, StringData} to simplify the code

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/428759 mentions this issue: runtime: replace stringStruct with unsafe.String where appropriate

gopherbot pushed a commit that referenced this issue Sep 13, 2022
@cuonglm @gopherbot
runtime: replace stringStruct with unsafe.String where appropriate
7ffbcd1 
Simplify the code a bit, no significant performance changes.

name                             old time/op    new time/op    delta
HashStringSpeed-8                  9.64ns ±11%    8.91ns ± 9%   -7.60%  (p=0.007 n=10+10)
HashStringArraySpeed-8             19.8ns ± 3%    19.5ns ± 2%     ~     (p=0.085 n=10+10)
MapStringKeysEight_16-8            10.7ns ± 3%    10.2ns ± 2%   -4.48%  (p=0.000 n=10+10)
MapStringKeysEight_32-8            8.89ns ± 3%    8.71ns ± 3%     ~     (p=0.082 n=9+10)
MapStringKeysEight_64-8            8.84ns ± 2%    8.60ns ± 3%   -2.73%  (p=0.004 n=9+10)
MapStringKeysEight_1M-8            8.90ns ± 3%    8.62ns ± 3%   -3.15%  (p=0.000 n=10+10)
MapStringConversion/32/simple-8    8.62ns ± 3%    8.61ns ± 2%     ~     (p=0.895 n=10+10)
MapStringConversion/32/struct-8    8.53ns ± 2%    8.63ns ± 2%     ~     (p=0.123 n=10+10)
MapStringConversion/32/array-8     8.54ns ± 2%    8.50ns ± 1%     ~     (p=0.590 n=9+9)
MapStringConversion/64/simple-8    8.44ns ± 2%    8.38ns ± 2%     ~     (p=0.353 n=10+10)
MapStringConversion/64/struct-8    8.41ns ± 2%    8.48ns ± 2%     ~     (p=0.143 n=10+10)
MapStringConversion/64/array-8     8.42ns ± 2%    8.44ns ± 2%     ~     (p=0.739 n=10+10)
MapInterfaceString-8               13.6ns ±26%    13.6ns ±20%     ~     (p=0.736 n=10+9)
AppendGrowString-8                 38.9ms ± 9%    40.2ms ±13%     ~     (p=0.481 n=10+10)
CompareStringEqual-8               3.03ns ± 2%    2.86ns ± 3%   -5.58%  (p=0.000 n=10+10)
CompareStringIdentical-8           1.20ns ± 3%    1.01ns ± 4%  -16.16%  (p=0.000 n=10+10)
CompareStringSameLength-8          2.11ns ± 3%    1.85ns ± 3%  -12.33%  (p=0.000 n=10+10)
CompareStringDifferentLength-8     0.30ns ± 0%    0.30ns ± 0%     ~     (p=0.508 n=10+9)
CompareStringBigUnaligned-8        43.0µs ± 1%    42.8µs ± 2%     ~     (p=0.165 n=10+10)
CompareStringBig-8                 43.2µs ± 2%    43.4µs ± 2%     ~     (p=0.661 n=9+10)
ConcatStringAndBytes-8             15.1ns ± 1%    14.9ns ± 1%   -1.57%  (p=0.001 n=8+10)
SliceByteToString/1-8              2.45ns ± 2%    2.39ns ± 2%   -2.64%  (p=0.000 n=10+10)
SliceByteToString/2-8              10.9ns ± 2%    10.8ns ± 4%     ~     (p=0.060 n=10+10)
SliceByteToString/4-8              11.9ns ± 0%    11.8ns ± 1%   -0.97%  (p=0.000 n=8+8)
SliceByteToString/8-8              13.9ns ± 1%    13.9ns ± 1%   +0.57%  (p=0.009 n=9+9)
SliceByteToString/16-8             18.0ns ± 3%    18.6ns ± 5%   +2.78%  (p=0.001 n=9+10)
SliceByteToString/32-8             20.1ns ± 3%    20.5ns ± 5%   +2.10%  (p=0.034 n=10+10)
SliceByteToString/64-8             24.3ns ± 3%    24.9ns ± 3%   +2.28%  (p=0.001 n=9+10)
SliceByteToString/128-8            33.8ns ± 1%    34.5ns ± 4%     ~     (p=0.264 n=8+10)

Updates #54854

Change-Id: I7ce57a92c5f590fa8cb31a48969d281147eb05f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/428759
Reviewed-by: hopehook <[email protected]>
Reviewed-by: Keith Randall <[email protected]>
Run-TryBot: Ian Lance Taylor <[email protected]>
Auto-Submit: Ian Lance Taylor <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Run-TryBot: Cuong Manh Le <[email protected]>
Reviewed-by: Ian Lance Taylor <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/443380 mentions this issue: runtime: avoid unsafe.String in debuglog

gopherbot pushed a commit that referenced this issue Oct 18, 2022
@prattmic @gopherbot
runtime: avoid unsafe.{Slice,String} in debuglog
c45ebef 
CL 428157 and CL 428759 switched debuglog to using unsafe.String and
unsafe.Slice, which broke the build with -tags=debuglog because this is
a no write barrier context, but runtime.unsafeString and unsafeSlice can
panic, which includes write barriers.

We could add a panicCheck1 path to these functions to reallow write
barriers, but it is a big mess to pass around the caller PC,
particularly since the compiler generates calls. It is much simpler to
just avoid unsafe.String and Slice.

Also add a basic test to build the runtime with -tags=debuglog to help
avoid future regressions.

For #54854.

Change-Id: I702418b986fbf189664e9aa4f40bc7de4d9e7781
Reviewed-on: https://go-review.googlesource.com/c/go/+/443380
Run-TryBot: Michael Pratt <[email protected]>
Auto-Submit: Michael Pratt <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/446396 mentions this issue: runtime: replace stringStruct with len and unsafe.StringData

romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Nov 3, 2022
@prattmic @romaindoumenc
runtime: avoid unsafe.{Slice,String} in debuglog
623eea7 
CL 428157 and CL 428759 switched debuglog to using unsafe.String and
unsafe.Slice, which broke the build with -tags=debuglog because this is
a no write barrier context, but runtime.unsafeString and unsafeSlice can
panic, which includes write barriers.

We could add a panicCheck1 path to these functions to reallow write
barriers, but it is a big mess to pass around the caller PC,
particularly since the compiler generates calls. It is much simpler to
just avoid unsafe.String and Slice.

Also add a basic test to build the runtime with -tags=debuglog to help
avoid future regressions.

For golang#54854.

Change-Id: I702418b986fbf189664e9aa4f40bc7de4d9e7781
Reviewed-on: https://go-review.googlesource.com/c/go/+/443380
Run-TryBot: Michael Pratt <[email protected]>
Auto-Submit: Michael Pratt <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Michael Knyszek <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@cuiweixie cuiweixie

Labels
NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
3 participants
@cuiweixie @hopehook @gopherbot