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

Go is going to lock down future uses of linkname #637

Closed
HeRaNO opened this issue May 25, 2024 · 8 comments · Fixed by #688
Closed

Go is going to lock down future uses of linkname #637

HeRaNO opened this issue May 25, 2024 · 8 comments · Fixed by #688
Labels
known-issue This issue is known to us, we are working on it

Comments

@HeRaNO
Copy link
Contributor

HeRaNO commented May 25, 2024

As is shown in golang/go#67401, Golang is going to lock down future uses of linkname and add sonic to the hall of shame due to the huge amounts of linkname use.

So maybe we need a -ldflags=-checklinkname=0 to make the compiling stage happy on Go 1.23. Or we can make a huge change to avoid using linkname.

Related CL:

@liuq19
Copy link
Collaborator

liuq19 commented May 27, 2024

Thanks much, we will investigate that

@stefanb
Copy link

stefanb commented Jun 26, 2024

The issue has already manifested when trying to build with Go 1.23 rc1 in homebrew:

@rsc
Copy link

rsc commented Jun 27, 2024

Hi. bytedance/sonic is widely used enough that we want to keep it working without changes, although of course we would be happy to see these linknames cleaned up in the future. We tried to allowlist all the symbols that bytedance/sonic needed. It looks like maybe we missed a couple? If you can identify the set of names we missed and file an issue in golang/go, I'd appreciate it. Thank you!

@AsterDY
Copy link
Collaborator

AsterDY commented Jun 27, 2024

Hi. bytedance/sonic is widely used enough that we want to keep it working without changes, although of course we would be happy to see these linknames cleaned up in the future. We tried to allowlist all the symbols that bytedance/sonic needed. It looks like maybe we missed a couple? If you can identify the set of names we missed and file an issue in golang/go, I'd appreciate it. Thank you!

@rsc Thx. After we manually move and copy some codes, the Go 1.23 compiler can build now - .#662.
However, some tests failed, perhaps due to the change of behavior of some links -- As usual .
If the Go team can build a notification mechanism to let us know which internal API we are using has changed, we will appreciate it a lot! It will save us a lot of time to match the new version.

@AsterDY AsterDY pinned this issue Jun 27, 2024
@AsterDY AsterDY added the known-issue This issue is known to us, we are working on it label Jun 28, 2024
@rsc
Copy link

rsc commented Jun 29, 2024

Thanks very much for the Go 1.23 fixes @AsterDY. We did not see the prof things you were changing via assembly.

In general the linknames that we have added for Go 1.23 will be left alone and not changed in the future, now that they are documented in our code base as being depended on by other projects. So ideally you should not need to update sonic again for Go 1.24 or future releases. Or at least you should not need to update them quite so often.

@AsterDY
Copy link
Collaborator

AsterDY commented Jul 3, 2024

Thanks very much for the Go 1.23 fixes @AsterDY. We did not see the prof things you were changing via assembly.

In general the linknames that we have added for Go 1.23 will be left alone and not changed in the future, now that they are documented in our code base as being depended on by other projects. So ideally you should not need to update sonic again for Go 1.24 or future releases. Or at least you should not need to update them quite so often.

BTW, is it possible to build sonic on Go1.23.rc2? @rsc

@haoxins
Copy link

haoxins commented Aug 14, 2024

this issue could be closed now~

@HeRaNO
Copy link
Contributor Author

HeRaNO commented Aug 14, 2024

this issue could be closed now~

I think we can wait for #688 to ensure having a full support.

SnowOnion added a commit to SnowOnion/godoogle that referenced this issue Sep 14, 2024
SnowOnion added a commit to SnowOnion/godoogle that referenced this issue Sep 17, 2024
* WIP: (may git squash!) feat: 1. init distance by weakening params / results 2. goimports + group (std + third party + current project)

* fix: arrow direction of 'weaken results' (WR)

* fix: 1. arrow direction of 'weaken results' (WR), 'weaken params' (WP); 2. Anonymize when building sigGraph

* feat: 1. Distance->DistanceWithCache => avg latency (200x same distance query) -60%; 2. (WP) distance 2->3 => query func(string)int, Atoi rank 17->4.

* feat: FloydWarshall; offline calc script

* feat: 1. parallel Floyd-Warshall; 2. Floyd-Warshall output dump to / load from file.

* tmp

* v0.0.4

* upgrade hertz to avoid bytedance/sonic#637

* floyd.json -> sigGraph.json

* NewHooglyRanker functional options

* HooglyRanker -> SigGraphRanker

* remove deadcode; polish webpage and testcase

* tailor tailor.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
known-issue This issue is known to us, we are working on it
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants