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

Tentative: Force compilation order #3915

Closed
wants to merge 1 commit into from
Closed

Tentative: Force compilation order #3915

wants to merge 1 commit into from

Conversation

kenbell
Copy link
Member

@kenbell kenbell commented Sep 18, 2023

It seems like the dependency logic in Build is broken. There's a bunch of code tracking actionid jobs - but nothing that seems to ensure a package is actually fully built before packages dependent on it.

In the world of templates, it seems like actually having dependencies fully built is important - we can't just rely on the linker to match up symbols later.

This branch is a quick hack to see if it fixes #3895 and a bunch of other build crashes by forcing compilation ordering. The build logic almost certainly needs revisiting more thoroughly - do we even need actionidjobs any more?

@kenbell kenbell mentioned this pull request Sep 18, 2023
Closed
@github-actions
Copy link

Size difference with the dev branch:

Binary size difference 
not the same command!
    go: downloading golang.org/x/text v0.3.6
    tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/ntpclient/main.go
not the same command!
    go: downloading github.com/eclipse/paho.mqtt.golang v1.2.0
    tinygo build -size short -o ./build/test.hex -target=wioterminal ./examples/rtl8720dn/mqttsub/
 before   after   diff
  60688   60688      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/adt7410/main.go
   9684    9684      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/adxl345/main.go
  13260   13260      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pybadge ./examples/amg88xx
   8776    8776      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/apa102/main.go
  11720   11720      0   0.00%  tinygo build -size short -o ./build/test.hex -target=nano-33-ble ./examples/apds9960/proximity/main.go
  10392   10392      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/apa102/itsybitsy-m0/main.go
   8196    8196      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/at24cx/main.go
   8308    8308      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bh1750/main.go
   7608    7608      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/blinkm/main.go
  63260   63260      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bmi160/main.go
  27868   27868      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bmp180/main.go
  63272   63272      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bmp280/main.go
  12432   12432      0   0.00%  tinygo build -size short -o ./build/test.hex -target=trinket-m0 ./examples/bmp388/main.go
   7708    7708      0   0.00%  tinygo build -size short -o ./build/test.hex -target=bluepill ./examples/ds1307/sram/main.go
  22008   22008      0   0.00%  tinygo build -size short -o ./build/test.hex -target=bluepill ./examples/ds1307/time/main.go
  68964   68964      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/ds3231/main.go
   4708    4708      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/easystepper/main.go
  24876   24876      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/espat/espconsole/main.go
  25104   25104      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/espat/esphub/main.go
  24860   24860      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/espat/espstation/main.go
  68684   68684      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/flash/console/spi
  64636   64636      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/flash/console/qspi
   7052    7052      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/gc9a01/main.go
  67956   67956      0   0.00%  tinygo build -size short -o ./build/test.hex -target=feather-m0 ./examples/gps/i2c/main.go
  68424   68424      0   0.00%  tinygo build -size short -o ./build/test.hex -target=feather-m0 ./examples/gps/uart/main.go
   8424    8424      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/hcsr04/main.go
   5712    5712      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/hd44780/customchar/main.go
   5756    5756      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/hd44780/text/main.go
  10616   10616      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/hd44780i2c/main.go
  14624   14624      0   0.00%  tinygo build -size short -o ./build/test.hex -target=nano-33-ble ./examples/hts221/main.go
  16904   16904      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/hub75/main.go
  10048   10048      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/basic
  10884   10884      0   0.00%  tinygo build -size short -o ./build/test.hex -target=xiao ./examples/ili9341/basic
  29044   29044      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/pyportal_boing
  10076   10076      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/scroll
  10960   10960      0   0.00%  tinygo build -size short -o ./build/test.hex -target=xiao ./examples/ili9341/scroll
 263524  263524      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/slideshow
  12092   12092      0   0.00%  tinygo build -size short -o ./build/test.hex -target=circuitplay-express ./examples/lis3dh/main.go
  14028   14028      0   0.00%  tinygo build -size short -o ./build/test.hex -target=nano-33-ble ./examples/lps22hb/main.go
  26124   26124      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/lsm303agr/main.go
  12576   12576      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/lsm6ds3/main.go
  10952   10952      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mag3110/main.go
  10200   10200      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp23017/main.go
  10628   10628      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp23017-multiple/main.go
   9868    9868      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp3008/main.go
  66772   66772      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp2515/main.go
  22952   22952      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/microbitmatrix/main.go
  22944   22944      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit-v2 ./examples/microbitmatrix/main.go
   8532    8532      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mma8653/main.go
   8316    8316      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mpu6050/main.go
  74988   74988      0   0.00%  tinygo build -size short -o ./build/test.hex -target=p1am-100 ./examples/p1am/main.go
  12152   12152      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pico ./examples/pca9685/main.go
   6084    6084      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/pcd8544/setbuffer/main.go
   5104    5104      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/pcd8544/setpixel/main.go
   2681    2681      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino ./examples/servo
   7944    7944      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pybadge ./examples/shifter/main.go
  56368   56368      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/sht3x/main.go
  56340   56340      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/shtc3/main.go
   6452    6452      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/ssd1306/i2c_128x32/main.go
   5940    5940      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/ssd1306/spi_128x64/main.go
   5704    5704      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/ssd1331/main.go
   6392    6392      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/st7735/main.go
   6292    6292      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/st7789/main.go
  17212   17212      0   0.00%  tinygo build -size short -o ./build/test.hex -target=circuitplay-express ./examples/thermistor/main.go
  10328   10328      0   0.00%  tinygo build -size short -o ./build/test.hex -target=circuitplay-bluefruit ./examples/tone
  10728   10728      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/tm1637/main.go
   9404    9404      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/touch/resistive/fourwire/main.go
  12460   12460      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/touch/resistive/pyportal_touchpaint/main.go
  15772   15772      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/vl53l1x/main.go
  13812   13812      0   0.00%  tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/vl6180x/main.go
   6460    6460      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/waveshare-epd/epd2in13/main.go
   6012    6012      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/waveshare-epd/epd2in13x/main.go
   6288    6288      0   0.00%  tinygo build -size short -o ./build/test.hex -target=microbit ./examples/waveshare-epd/epd4in2/main.go
 137184  137184      0   0.00%  go: downloading golang.org/x/text v0.3.6
 137176  137176      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/udpstation/main.go
 137500  137500      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/tcpclient/main.go
 137600  137600      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/wifinina/webclient/main.go
   6976    6976      0   0.00%  tinygo build -size short -o ./build/test.hex -target=circuitplay-express ./examples/ws2812
   5326    5326      0   0.00%  tinygo build -size short -o ./build/test.bin -target=m5stamp-c3          ./examples/ws2812
  61656   61656      0   0.00%  tinygo build -size short -o ./build/test.hex -target=feather-nrf52840 ./examples/is31fl3731/main.go
   1565    1565      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino   ./examples/ws2812
    880     880      0   0.00%  tinygo build -size short -o ./build/test.hex -target=digispark ./examples/ws2812
  32296   32296      0   0.00%  tinygo build -size short -o ./build/test.hex -target=trinket-m0 ./examples/bme280/main.go
  16660   16660      0   0.00%  tinygo build -size short -o ./build/test.hex -target=circuitplay-express ./examples/microphone/main.go
  11336   11336      0   0.00%  tinygo build -size short -o ./build/test.hex -target=circuitplay-express ./examples/buzzer/main.go
  13044   13044      0   0.00%  tinygo build -size short -o ./build/test.hex -target=trinket-m0 ./examples/veml6070/main.go
   6868    6868      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/l293x/simple/main.go
   8788    8788      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/l293x/speed/main.go
   6832    6832      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/l9110x/simple/main.go
   9448    9448      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/l9110x/speed/main.go
   7344    7344      0   0.00%  tinygo build -size short -o ./build/test.hex -target=nucleo-f103rb ./examples/shiftregister/main.go
   7084    7084      0   0.00%  tinygo build -size short -o ./build/test.hex -target=hifive1b ./examples/ssd1351/main.go
  13224   13224      0   0.00%  tinygo build -size short -o ./build/test.hex -target=circuitplay-express ./examples/lis2mdl/main.go
   8472    8472      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/max72xx/main.go
  77416   77416      0   0.00%  tinygo build -size short -o ./build/test.hex -target=feather-m0 ./examples/dht/main.go
  70928   70928      0   0.00%  tinygo build -size short -o ./build/test.hex -target=xiao ./examples/pcf8563/alarm/
   7452    7452      0   0.00%  tinygo build -size short -o ./build/test.hex -target=xiao ./examples/pcf8563/clkout/
  70440   70440      0   0.00%  tinygo build -size short -o ./build/test.hex -target=xiao ./examples/pcf8563/time/
  70800   70800      0   0.00%  tinygo build -size short -o ./build/test.hex -target=xiao ./examples/pcf8563/timer/
  12096   12096      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pico ./examples/qmi8658c/main.go
   8968    8968      0   0.00%  tinygo build -size short -o ./build/test.hex -target=feather-m0 ./examples/ina260/main.go
   9280    9280      0   0.00%  tinygo build -size short -o ./build/test.hex -target=nucleo-l432kc ./examples/aht20/main.go
  72008   72008      0   0.00%  tinygo build -size short -o ./build/test.hex -target=feather-m4 ./examples/sdcard/console/
  82532   82532      0   0.00%  tinygo build -size short -o ./build/test.hex -target=wioterminal ./examples/rtl8720dn/webclient/
  71924   71924      0   0.00%  tinygo build -size short -o ./build/test.hex -target=wioterminal ./examples/rtl8720dn/webserver/
  98452   98452      0   0.00%  go: downloading github.com/eclipse/paho.mqtt.golang v1.2.0
  60292   60292      0   0.00%  tinygo build -size short -o ./build/test.hex -target=feather-m4 ./examples/i2csoft/adt7410/
  10164   10164      0   0.00%  tinygo build -size short -o ./build/test.elf -target=wioterminal ./examples/axp192/m5stack-core2-blinky/
   8924    8924      0   0.00%  tinygo build -size short -o ./build/test.uf2 -target=pico ./examples/xpt2046/main.go
  14560   14560      0   0.00%  tinygo build -size short -o ./build/test.hex -target=nucleo-wl55jc ./examples/sx126x/lora_rxtx/
  25876   25876      0   0.00%  tinygo build -size short -o ./build/test.uf2 -target=pico ./examples/ssd1289/main.go
  11204   11204      0   0.00%  tinygo build -size short -o ./build/test.hex -target=pico ./examples/irremote/main.go
  11220   11220      0   0.00%  tinygo build -size short -o ./build/test.hex -target=badger2040 ./examples/uc8151/main.go
  10308   10308      0   0.00%  tinygo build -size short -o ./build/test.uf2 -target=pico ./examples/scd4x/main.go
   8744    8744      0   0.00%  tinygo build -size short -o ./build/test.uf2 -target=circuitplay-express ./examples/makeybutton/main.go
   9608    9608      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/ds18b20/main.go
  81220   81220      0   0.00%  tinygo build -size short -o ./build/test.hex -target=nucleo-wl55jc ./examples/lora/lorawan/atcmd/
  15740   15740      0   0.00%  tinygo build -size short -o ./build/test.uf2 -target=pico ./examples/as560x/main.go
   9800    9800      0   0.00%  tinygo build -size short -o ./build/test.uf2 -target=pico ./examples/mpu6886/main.go
   7892    7892      0   0.00%  tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/ttp229/main.go
3459024 3459024      0   0.00%   sum

@aykevl
Copy link
Member

aykevl commented Sep 18, 2023

In the world of templates

I think you mean generics?

it seems like actually having dependencies fully built is important - we can't just rely on the linker to match up symbols later.

It's slightly more complicated than that. Even if packages are fully built before their dependencies are, there can be race conditions. I still suspect this is a bug in the x/tools/go/ssa package, but writing a reproducer is hard so that's why I've been postponing it. For more information, see: #3532 (in this reproducer, two packages that import the same dependency result in a race condition).

But to clarify: there is nothing on the TinyGo side that requires the dependencies to be fully built. And in fact, this worked just fine before we added support for generics. Hence why this seems like a x/tools/go/ssa bug to me.

I've looked into this recently again. One fix that appears to be working reliably is building the Go SSA serially, like this:

@@ -344,13 +347,47 @@ func Build(pkgName, outpath, tmpdir string, config *compileopts.Config) (BuildRe
                }
                packageActionIDJobs[pkg.ImportPath] = packageActionIDJob
 
+               // Build the SSA for the given package.
+               ssaPkg := program.Package(pkg.Pkg)
+               ssaPkg.Build()

...but that slows down the build and also builds packages that don't need to be rebuilt. There must be a better way to do this though, but it's kind of complicated to get right and not do unnecessary work.

dkegel-fastly added a commit that referenced this pull request Sep 20, 2023
@dkegel-fastly
build: build Go SSA serially [issue 3895]
75f2760 
From
#3915 (comment)

According to Ayke, it slows down the build but seems to reliably fix #3895
@dkegel-fastly dkegel-fastly mentioned this pull request Sep 20, 2023
Merged
deadprogram pushed a commit that referenced this pull request Sep 20, 2023
@dkegel-fastly @deadprogram
build: build Go SSA serially [issue 3895]
13a8eae 
From
#3915 (comment)

According to Ayke, it slows down the build but seems to reliably fix #3895
@kenbell
Copy link
Member Author

kenbell commented Sep 21, 2023

Closing as issue fixed by #3920

@kenbell kenbell closed this Sep 21, 2023
@kenbell kenbell deleted the force-compilation-order branch September 21, 2023 04:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kenbell @aykevl