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.

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

chore(CI): build a portable binary in CI for different CPU architectures #1504

Merged
merged 2 commits into from
Oct 27, 2023
Merged

chore(CI): build a portable binary in CI for different CPU architectures #1504

merged 2 commits into from
Oct 27, 2023

Conversation

Flouse
Copy link
Contributor

@Flouse Flouse commented Oct 27, 2023 •
edited
Loading

What this PR does / why we need it?

This PR tries to:

PORTABLE=1: This flag is used to build a portable binary that can run on different machines with different CPU architectures. It disables the use of CPU-specific optimizations and generates code that is compatible with a wide range of CPUs

USE_SSE=1: This flag enables the use of SSE instructions, which are a set of CPU instructions that can perform multiple operations on multiple data elements in parallel. SSE instructions can significantly improve performance for certain types of computations, such as multimedia processing

What is the impact of this PR?

No Breaking Change

CI Settings

CI Usage

Tip: Check the CI you want to run below, and then comment /run-ci.

CI Switch

  • Web3 Compatible Tests
  • OCT 1-5 And 12-15
  • OCT 6-10
  • OCT 11
  • OCT 16-19
  • v3 Core Tests

CI Description

CI Name Description
Web3 Compatible Test Test the Web3 compatibility of Axon
v3 Core Test Run the compatibility tests provided by Uniswap V3
OCT 1-5 | 6-10 | 11 | 12-15 | 16-19 Run the compatibility tests provided by OpenZeppelin

@Flouse
Copy link
Contributor Author

Flouse commented Oct 27, 2023

/run-ci

@github-actions
Copy link

CI tests run on commit:

CI test list:

  • OCT 1-5 And 12-15
  • OCT 6-10
  • OCT 11
  • OCT 16-19
  • v3 Core Tests
  • Web3 Compatible Tests

Please check ci test results later.

@Flouse Flouse marked this pull request as ready for review October 27, 2023 02:48
@Flouse Flouse requested a review from a team as a code owner October 27, 2023 02:48
@Flouse Flouse requested review from ashuralyk, blckngm and KaoImin and removed request for ashuralyk October 27, 2023 02:48
blckngm
blckngm reviewed Oct 27, 2023
View reviewed changes
Copy link
Contributor

@blckngm blckngm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should we use -march=native? I thought the change would be to make ckb-rocksdb use a more portable CPU feature set.

@KaoImin KaoImin changed the title chore(CI): RocksDB uses -march=native, Axon uses it too chore(CI): RocksDB uses -march=native Axon uses it too Oct 27, 2023
KaoImin
KaoImin previously approved these changes Oct 27, 2023
View reviewed changes
@Flouse
Copy link
Contributor Author

Flouse commented Oct 27, 2023

Why should we use -march=native? I thought the change would be to make ckb-rocksdb use a more portable CPU feature set.

rustc -C target-cpu=help
Available CPUs for this target:
    native         - Select the CPU of the current host

@blckngm
Copy link
Contributor

blckngm commented Oct 27, 2023

Using -march=native may cause the same issue that cached objects may use instructions that the current host doesn't support. We should change rocksdb to not use -march=native, not change rust to use -march=native.

@Flouse Flouse changed the title chore(CI): RocksDB uses -march=native Axon uses it too chore(CI): build a portable binary in CI for different CPU architectures Oct 27, 2023
@Flouse Flouse requested a review from blckngm October 27, 2023 04:14
@Flouse Flouse enabled auto-merge October 27, 2023 04:15
@Flouse Flouse requested a review from KaoImin October 27, 2023 04:15
@Flouse Flouse requested a review from driftluo October 27, 2023 04:19
@blckngm
Copy link
Contributor

blckngm commented Oct 27, 2023

Ckb-rocksdb uses PORTABLE=1, USE_SSE=1 by default so this change isn't necessary. If there are runners that don't support avx2, we should use the portable feature of ckb-rocksdb to disable avx2.

@Flouse Flouse added this pull request to the merge queue Oct 27, 2023
Merged via the queue into main with commit 363b49f Oct 27, 2023
18 of 19 checks passed
@Flouse Flouse deleted the fix-cargo-build-in-ci branch October 27, 2023 07:53
@Flouse Flouse mentioned this pull request Oct 27, 2023
Closed
KaoImin pushed a commit that referenced this pull request Nov 17, 2023
@Flouse @KaoImin
chore(CI): build a portable binary in CI for different CPU architectu…
249bff6 
…res (#1504)

* chore(CI): RocksDB uses `-march=native`, Axon uses it too

* chore(CI): build a portable binary in CI for different CPU architectures
@yangby-cryptape yangby-cryptape mentioned this pull request Nov 17, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@KaoImin KaoImin KaoImin approved these changes

@blckngm blckngm blckngm approved these changes

@driftluo driftluo Awaiting requested review from driftluo

Assignees
No one assigned
Labels
chore skip_changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CI may be broken by illegal instruction
3 participants
@Flouse @blckngm @KaoImin