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

Implement Trylock #3983

Merged
merged 1 commit into from
Jan 26, 2024
Merged

Implement Trylock #3983

merged 1 commit into from
Jan 26, 2024

Conversation

soypat
Copy link
Contributor

@soypat soypat commented Nov 2, 2023

I also fixed something which seems like a bug... The unlock method was not resetting the value of the locked field. I suspect there may be more than meets the eye and that this is not a bug...

I now understand what the code is doing, removed the bug I introduced via force push 🤦

@soypat soypat requested a review from aykevl November 2, 2023 00:19
@soypat soypat changed the base branch from release to dev November 2, 2023 00:20
Copy link

github-actions bot commented Nov 2, 2023

Size difference with the dev branch:

Binary size difference
 flash                          ram
 before   after   diff          before   after   diff
  22920   22884    -36  -0.16%    2424    2424      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/microbitmatrix/main.go
  22912   22892    -20  -0.09%    4468    4468      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit-v2 ./examples/microbitmatrix/main.go
  22096   22088     -8  -0.04%    3528    3528      0   0.00% tinygo build -size short -o ./build/test.hex -target=bluepill ./examples/ds1307/time/main.go
  60736   60736      0   0.00%    6160    6160      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/adt7410/main.go
   9612    9612      0   0.00%    4740    4740      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/adxl345/main.go
  13232   13232      0   0.00%    6780    6780      0   0.00% tinygo build -size short -o ./build/test.hex -target=pybadge ./examples/amg88xx
   8600    8600      0   0.00%    4732    4732      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/apa102/main.go
  11584   11584      0   0.00%    6556    6556      0   0.00% tinygo build -size short -o ./build/test.hex -target=nano-33-ble ./examples/apds9960/proximity/main.go
   9660    9660      0   0.00%    4752    4752      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/apa102/itsybitsy-m0/main.go
   8180    8180      0   0.00%    2304    2304      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/at24cx/main.go
   8220    8220      0   0.00%    4732    4732      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bh1750/main.go
   7508    7508      0   0.00%    4732    4732      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/blinkm/main.go
  70220   70220      0   0.00%    3676    3676      0   0.00% tinygo build -size short -o ./build/test.hex -target=pinetime     ./examples/bma42x/main.go
  63156   63156      0   0.00%    6160    6160      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bmi160/main.go
  27644   27644      0   0.00%    4772    4772      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bmp180/main.go
  63264   63264      0   0.00%    6192    6192      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/bmp280/main.go
  12212   12212      0   0.00%    4804    4804      0   0.00% tinygo build -size short -o ./build/test.hex -target=trinket-m0 ./examples/bmp388/main.go
   8128    8128      0   0.00%    3332    3332      0   0.00% tinygo build -size short -o ./build/test.hex -target=bluepill ./examples/ds1307/sram/main.go
  69044   69044      0   0.00%    6332    6332      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/ds3231/main.go
   4704    4704      0   0.00%    2264    2264      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/easystepper/main.go
  24792   24792      0   0.00%    6292    6292      0   0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/espat/espconsole/main.go
  24940   24940      0   0.00%    6292    6292      0   0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/espat/esphub/main.go
  24792   24792      0   0.00%    6292    6292      0   0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/espat/espstation/main.go
  68620   68620      0   0.00%    6940    6940      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/flash/console/spi
  64652   64652      0   0.00%    8980    8980      0   0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/flash/console/qspi
   7040    7040      0   0.00%    2264    2264      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/gc9a01/main.go
  67468   67468      0   0.00%    6336    6336      0   0.00% tinygo build -size short -o ./build/test.hex -target=feather-m0 ./examples/gps/i2c/main.go
  67896   67896      0   0.00%    6472    6472      0   0.00% tinygo build -size short -o ./build/test.hex -target=feather-m0 ./examples/gps/uart/main.go
   8260    8260      0   0.00%    4732    4732      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/hcsr04/main.go
   5612    5612      0   0.00%    2264    2264      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/hd44780/customchar/main.go
   5656    5656      0   0.00%    2264    2264      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/hd44780/text/main.go
  10440   10440      0   0.00%    4740    4740      0   0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/hd44780i2c/main.go
  14496   14496      0   0.00%    6556    6556      0   0.00% tinygo build -size short -o ./build/test.hex -target=nano-33-ble ./examples/hts221/main.go
  16868   16868      0   0.00%    2344    2344      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/hub75/main.go
  10056   10056      0   0.00%    6900    6900      0   0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/basic
  10696   10696      0   0.00%    4852    4852      0   0.00% tinygo build -size short -o ./build/test.hex -target=xiao ./examples/ili9341/basic
  28996   28996      0   0.00%   38060   38060      0   0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/pyportal_boing
  10080   10080      0   0.00%    6908    6908      0   0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/scroll
  10784   10784      0   0.00%    4860    4860      0   0.00% tinygo build -size short -o ./build/test.hex -target=xiao ./examples/ili9341/scroll
  11784   11784      0   0.00%    4772    4772      0   0.00% tinygo build -size short -o ./build/test.hex -target=circuitplay-express ./examples/lis3dh/main.go
  13840   13840      0   0.00%    6556    6556      0   0.00% tinygo build -size short -o ./build/test.hex -target=nano-33-ble ./examples/lps22hb/main.go
  26052   26052      0   0.00%    2312    2312      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/lsm303agr/main.go
  12360   12360      0   0.00%    4780    4780      0   0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/lsm6ds3/main.go
  10800   10800      0   0.00%    4732    4732      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mag3110/main.go
  10004   10004      0   0.00%    4764    4764      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp23017/main.go
  10424   10424      0   0.00%    4772    4772      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp23017-multiple/main.go
   9692    9692      0   0.00%    4732    4732      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp3008/main.go
  66752   66752      0   0.00%    6160    6160      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mcp2515/main.go
   8320    8320      0   0.00%    4740    4740      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mma8653/main.go
   8232    8232      0   0.00%    4732    4732      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/mpu6050/main.go
  74908   74908      0   0.00%    7444    7444      0   0.00% tinygo build -size short -o ./build/test.hex -target=p1am-100 ./examples/p1am/main.go
  12120   12120      0   0.00%    3336    3336      0   0.00% tinygo build -size short -o ./build/test.hex -target=pico ./examples/pca9685/main.go
   6068    6068      0   0.00%    3272    3272      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/pcd8544/setbuffer/main.go
   5104    5104      0   0.00%    2264    2264      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/pcd8544/setpixel/main.go
   2697    2697      0   0.00%     556     556      0   0.00% tinygo build -size short -o ./build/test.hex -target=arduino ./examples/servo
   7940    7940      0   0.00%    6780    6780      0   0.00% tinygo build -size short -o ./build/test.hex -target=pybadge ./examples/shifter/main.go
  56400   56400      0   0.00%    3652    3652      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/sht3x/main.go
  56456   56456      0   0.00%    3660    3660      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/sht4x/main.go
  56372   56372      0   0.00%    3652    3652      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/shtc3/main.go
   6460    6460      0   0.00%    2272    2272      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/ssd1306/i2c_128x32/main.go
   5964    5964      0   0.00%    2264    2264      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/ssd1306/spi_128x64/main.go
   5676    5676      0   0.00%    2264    2264      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/ssd1331/main.go
   6376    6376      0   0.00%    2264    2264      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/st7735/main.go
   6232    6232      0   0.00%    2264    2264      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/st7789/main.go
  17044   17044      0   0.00%    4732    4732      0   0.00% tinygo build -size short -o ./build/test.hex -target=circuitplay-express ./examples/thermistor/main.go
  10292   10292      0   0.00%    4516    4516      0   0.00% tinygo build -size short -o ./build/test.hex -target=circuitplay-bluefruit ./examples/tone
   9972    9972      0   0.00%    4732    4732      0   0.00% tinygo build -size short -o ./build/test.hex -target=arduino-nano33 ./examples/tm1637/main.go
   9396    9396      0   0.00%    6772    6772      0   0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/touch/resistive/fourwire/main.go
  12468   12468      0   0.00%    6984    6984      0   0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/touch/resistive/pyportal_touchpaint/main.go
  15708   15708      0   0.00%    4740    4740      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/vl53l1x/main.go
  13736   13736      0   0.00%    4740    4740      0   0.00% tinygo build -size short -o ./build/test.hex -target=itsybitsy-m0 ./examples/vl6180x/main.go
   6432    6432      0   0.00%    2304    2304      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/waveshare-epd/epd2in13/main.go
   6004    6004      0   0.00%    2296    2296      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/waveshare-epd/epd2in13x/main.go
   6260    6260      0   0.00%    2304    2304      0   0.00% tinygo build -size short -o ./build/test.hex -target=microbit ./examples/waveshare-epd/epd4in2/main.go
 263056  263072     16   0.01%   46720   46720      0   0.00% tinygo build -size short -o ./build/test.hex -target=pyportal ./examples/ili9341/slideshow
1873177 1873129    -48  -0.00%  417624  417624      0   0.00%

// and use of TryLock is often a sign of a deeper problem
// in a particular use of mutexes.
func (m *Mutex) TryLock() bool {
if m.locked {
Copy link
Member

Choose a reason for hiding this comment

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

It wouldn't surprise me if we needed calls to volatile loads here: https://github.com/tinygo-org/tinygo/blob/release/src/runtime/volatile/volatile.go

Copy link
Contributor Author

Choose a reason for hiding this comment

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

🤔 Wouldn't a memory barrier be better suited for this use case? Even then, tinygo is single threaded so we should have consistent ordering?

Copy link
Member

Choose a reason for hiding this comment

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

I'm more concerned about LLVM removing loads/unloads it doesn't think are important. Yes, TInyGo is single threaded so memory barriers are less interesting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added volatile loads/stores as requested!

@soypat soypat requested a review from dgryski November 19, 2023 04:58
@deadprogram
Copy link
Member

Looking at this code, seems like it should be OK to squash/merge.

@soypat do you want to squash the commits yourself?
@aykevl or anyone else any other feedback before merge?

Copy link
Member

@aykevl aykevl left a comment

Choose a reason for hiding this comment

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

Looks good to me, though if we're going to care about atomicity we should really use the sync/atomic package instead. Volatile has slightly different semantics (though for most microcontrollers it doesn't really matter).

Rebased on the dev branch, ready to merge once CI passes.

@aykevl aykevl merged commit cb39611 into dev Jan 26, 2024
23 of 24 checks passed
@aykevl aykevl deleted the trylock branch January 26, 2024 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants