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

MPSC count can become negative #49

Closed
mratsim opened this issue Dec 15, 2019 · 2 comments · Fixed by #138
Closed

MPSC count can become negative #49

mratsim opened this issue Dec 15, 2019 · 2 comments · Fixed by #138

Comments

@mratsim
Copy link
Owner

mratsim commented Dec 15, 2019

Even though the count is done on the consumer side which should always underestimate the real count, the number of estimated enqueued item in the MPSC channel can be negative.

See #48 (comment) and CI https://dev.azure.com/numforge/Weave/_build/results?buildId=25&view=logs

This is not blocking as this count is only informative and used for adaptative stealing and for the memory pool to trigger batch reception of memory.

This is something I actualy remarked in the past but seems to happen rarely:

func peek*(chan: var ChannelMpscUnboundedBatch): int32 {.inline.} =
## Estimates the number of items pending in the channel
## - If called by the consumer the true number might be more
## due to producers adding items concurrently.
## - If called by a producer the true number is undefined
## as other producers also add items concurrently and
## the consumer removes them concurrently.
##
## This is a non-locking operation.
result = int32 chan.count.load(moAcquire)
# For the consumer it's always positive or zero
postCondition: result >= 0 # TODO somehow it can be -1

mratsim added a commit that referenced this issue Dec 15, 2019
* Try to fix the "deadlock" in batch receive, somehow it's also 5% faster (to be checked in Travis, deadlock not reproduced locally)

* Ignore nimble during runs nim-lang/nimble#696

* don't auto fail due to getTicks on ARM

* Negative count is non-blocking, can be investigated later (#49)

* When using C++ we need C++11

* typo

* Make travis Arch visible

* fix arch
@mratsim
Copy link
Owner Author

mratsim commented May 9, 2020

Hopefully solved by partial model checking in #128 (memory bug on the model checker that interrupts it, ...)

@mratsim mratsim closed this as completed May 9, 2020
@mratsim
Copy link
Owner Author

mratsim commented May 16, 2020

Still happening in the memory pool:

========================================================================================
Running [ c  ] weave/memory/memory_pools.nim
========================================================================================
Single-threaded: System alloc for 100 blocks: 0.9514 s
Single-threaded: Pool   alloc for 100 blocks: 0.4980 s
Multi-threaded: System alloc: 0.0331 s
fatal.nim(49)            sysFatal
Error: unhandled exception: contracts.nim(86, 15) `
0 <= chan.count.load(moRelaxed)` 
    Contract violated for post-condition at channels_mpsc_unbounded_batch.nim:250
        
0 <= chan.count.load(moRelaxed)
    The following values are contrary to expectations:
        
0 <= chan.count.load(moRelaxed)  [Worker N/A]
 [AssertionError]
Error: execution of an external program failed: '/home/vsts/work/1/s/build/memory_pools '
stack trace: (most recent call last)
/tmp/nimblecache/nimscriptapi.nim(165, 16)
/home/vsts/work/1/s/weave_5847.nims(46, 8) testTask
/home/vsts/work/1/s/weave_5847.nims(32, 8) test
/home/vsts/work/1/s/NimBinaries/nim-stable/lib/system/nimscript.nim(260, 7) exec
/home/vsts/work/1/s/NimBinaries/nim-stable/lib/system/nimscript.nim(260, 7) Error: unhandled exception: FAILED: nim c  --verbosity:0 --hints:off --warnings:off --threads:on -d:release --outdir:build -r weave/memory/memory_pools.nim [OSError]
       Tip: 1 messages have been suppressed, use --verbose to show them.
     Error: Exception raised during nimble script execution

@mratsim mratsim reopened this May 16, 2020
mratsim added a commit that referenced this issue May 16, 2020
mratsim added a commit that referenced this issue May 16, 2020
mratsim added a commit that referenced this issue May 16, 2020
* Change the mpsc counting to fix #49

* Make MPSC count optional, finishes #93

* semantics: ascertain -> postCondition

* rebase
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant