OpenEugene / openboard

An open source switchboard, written in Elm and Go.
MIT License
22 stars 7 forks source link

Logging policy debug sync atomic uint32 version-123 #161

Closed codegold79 closed 3 years ago

codegold79 commented 3 years ago

Here is an example where we use the sync/atomic packages's load/store uint32 types.

codegold79 commented 3 years ago

Benchmark test results

Range: 1.085s - 1.479s Average: 1.333s

TEST A

   10000            126837 ns/op
PASS
ok      github.com/OpenEugene/openboard/back/internal/dbg       1.275s

TEST B

   10000            145361 ns/op
PASS
ok      github.com/OpenEugene/openboard/back/internal/dbg       1.464s

TEST C

   10000            134445 ns/op
PASS
ok      github.com/OpenEugene/openboard/back/internal/dbg       1.360s

TEST D

   10000            146970 ns/op
PASS
ok      github.com/OpenEugene/openboard/back/internal/dbg       1.479s

TEST E

   10000            107690 ns/op
PASS
ok      github.com/OpenEugene/openboard/back/internal/dbg       1.085s
codegold79 commented 3 years ago

Benchmark tests show that sync/atomic using uint32 has the best performance compared to the other two ways I've tried. I recommend going with this PR and I've labeled the other two, [NOT FOR MERGE].

This PR is ready for review.

codegold79 commented 3 years ago

After improving benchmark test, I ran test again:

20482 79570 ns/op PASS ok github.com/OpenEugene/openboard/back/internal/dbg 2.230s

codegold79 commented 3 years ago

@daved, now that I improved the benchmark test and ran tests again, this version (sync/atomic uint32) happens to be the worst and mutex the best. I didn't run as many tests, but the difference seems to be minimal and I like this sync/atomic uint32 way personally.

codegold79 commented 3 years ago

During our last meeting, @daved realized that this version has a race condition where d.logln and d.logf might not be protected from a race condition, even though d.toggle is protected.

We'll have to choose from the other sync atomic version or the mutex version.

Closing PR.