-
-
Notifications
You must be signed in to change notification settings - Fork 122
add atomic global counter system #1323
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
base: master
Are you sure you want to change the base?
Changes from all commits
dcbc141
df0477a
6a35f59
b785c82
adcec68
aaf0b25
ae8b776
d949beb
89214fa
6024eaa
d9e4b00
3ac82ec
9a73bd4
b5f675d
a7fad51
e872f37
8bb2038
abdbf7f
7c4b6b7
fbc5351
0298263
2ab5cb1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| Fixed global counter system using an atomic variable | ||
| -- by :user:`Vizonex`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| import sysconfig | ||
| import threading | ||
|
|
||
| import multidict | ||
Check noticeCode scanning / CodeQL Module is imported with 'import' and 'import from' Note test
Module 'multidict' is imported with both 'import' and 'import from'.
|
||
|
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should probably resolve this, no reason to import it twice.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Concur — easiest fix is |
||
| from multidict import MultiDict | ||
|
|
||
| FREETHREADED = bool(sysconfig.get_config_var("Py_GIL_DISABLED")) | ||
|
|
||
|
|
||
| md: MultiDict[int] = MultiDict() | ||
| N, M = 3, 100 | ||
| baseline = multidict.getversion(md) # type: ignore[arg-type] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why ignore?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If there's an error here, then maybe my strict |
||
|
|
||
|
|
||
| def worker(tid: int) -> None: | ||
| for i in range(M): | ||
| md[f"k{tid}_{i}"] = i | ||
|
|
||
|
|
||
| if (__name__ == "__main__") and FREETHREADED: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same concern as in other PRs: we shouldn't need to gate on FT.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreeing with this concern. Now that |
||
| threads = [threading.Thread(target=worker, args=(tid,)) for tid in range(N)] | ||
| for t in threads: | ||
| t.start() | ||
| for t in threads: | ||
| t.join() | ||
|
|
||
| observed = multidict.getversion(md) - baseline # type: ignore[arg-type] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why ignore?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| expected = N * M | ||
| assert expected == observed, ( | ||
| f"expected delta: {expected}" | ||
| f" observed: {observed} " | ||
| f"lost: {expected - observed}" | ||
| ) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MSVC handling currently prefixes
BASE_CFLAGSvalues with/(e.g.O0->/O0,g3->/g3). These aren't valid MSVC flags (/Odand/Zi//Z7are the usual equivalents), so Windows builds withMULTIDICT_DEBUG_BUILD=1will fail. Consider defining a separate MSVC-specific debug/release flag list instead of reusingBASE_CFLAGSverbatim.