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

fix: always build for amd64 when building go functions #1121

Merged
merged 1 commit into from Jun 24, 2022
Merged

Conversation

danez
Copy link
Contributor

@danez danez commented Jun 23, 2022

I noticed that we build for whatever architecture the current host has. Which when for example building locally on M1 MacBook probably creates Linux/arm64 binary which would be wrong, but even if not I feel saver by defining it explicitly.

Ref: https://github.com/netlify/pillar-runtime/issues/231

@danez danez added the type: bug code to address defects in shipped code label Jun 23, 2022
@danez danez requested a review from a team June 23, 2022 16:31
@github-actions
Copy link
Contributor

⏱ Benchmark results

Comparing with 5083bb2

largeDepsEsbuild: 6.9s

⬇️ 22.50% decrease vs. 5083bb2

^   8.4s                          
│   ┌──┐                          
│   |  |                          
│   |  |                          
│   |  |                    6.9s  
│ ──┼──┼────────────6.5s────┌──┐──
│   |  |            ┌──┐    |▒▒|  
│   |  |    5.6s    |  |    |▒▒|  
│   |  |    ┌──┐    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴──>
    T-3     T-2     T-1      T    
Legend

largeDepsNft: 34.2s

⬇️ 26.29% decrease vs. 5083bb2

^  43.2s                          
│   ┌──┐                          
│   |  |                          
│   |  |                          
│   |  |                   34.2s  
│ ──┼──┼────────────────────┌──┐──
│   |  |           29.6s    |▒▒|  
│   |  |    27s     ┌──┐    |▒▒|  
│   |  |    ┌──┐    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
│   |  |    |  |    |  |    |▒▒|  
└───┴──┴────┴──┴────┴──┴────┴──┴──>
    T-3     T-2     T-1      T    
Legend

largeDepsZisi: 51.7s

^                          51.7s  
│                           ┌──┐  
│                   46s     |▒▒|  
│ ──────────────────┌──┐────|▒▒|──
│          41.7s    |  |    |▒▒|  
│           ┌──┐    |  |    |▒▒|  
│           |  |    |  |    |▒▒|  
│           |  |    |  |    |▒▒|  
│           |  |    |  |    |▒▒|  
│           |  |    |  |    |▒▒|  
│           |  |    |  |    |▒▒|  
│           |  |    |  |    |▒▒|  
│           |  |    |  |    |▒▒|  
│           |  |    |  |    |▒▒|  
│           |  |    |  |    |▒▒|  
│           |  |    |  |    |▒▒|  
│           |  |    |  |    |▒▒|  
│           |  |    |  |    |▒▒|  
│           |  |    |  |    |▒▒|  
│           |  |    |  |    |▒▒|  
│           |  |    |  |    |▒▒|  
│   n/a     |  |    |  |    |▒▒|  
└───────────┴──┴────┴──┴────┴──┴──>
    T-3     T-2     T-1      T    
Legend

Copy link
Member

@eduardoboucas eduardoboucas left a comment

Choose a reason for hiding this comment

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

Maybe it'd be nice to make this configurable in the future, in case we start using a different arch in Lambda (like ARM).

This is great for now, though! ✨

@danez danez merged commit 4ba670c into main Jun 24, 2022
@danez danez deleted the Set-GOARCH branch June 24, 2022 10:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants