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

Threads API in Standard Library #1333

Draft
wants to merge 17 commits into
base: main
Choose a base branch
from
Draft

Conversation

garlic0x1
Copy link
Contributor

I have added a wrapper over bordeaux-threads to the standard library, it is only loaded if this reader conditional is satisfied: #+(or sb-thread threads openmcl-native-threads), which tests for thread capabilities in SBCL, ECL, and CCL.

This might not be in scope for the standard library but I think it will be useful.

@garlic0x1 garlic0x1 changed the title Threads API in Standard Library Threads API in Standard Library (WIP) Dec 26, 2024
@garlic0x1 garlic0x1 marked this pull request as draft December 26, 2024 20:48
@garlic0x1 garlic0x1 marked this pull request as ready for review December 27, 2024 02:32
@garlic0x1
Copy link
Contributor Author

known deficiencies:

  • Some options like names and timeouts aren't yet exposed as I'm not sure the idiomatic way to compensate for lack of kwargs.
  • Only tested on SBCL and CCL.

possible style issues:

  • Names are slightly garlicized.
  • with-lock-held should probably be changed, other with macros in the stdlib are actually call-with macros so I am unsure of the naming convention.

@garlic0x1 garlic0x1 changed the title Threads API in Standard Library (WIP) Threads API in Standard Library Dec 27, 2024
@stylewarning
Copy link
Member

@garlic0x1 What do you think about having a separate ASDF subsystem for this? Not 100% sure, but maybe would be a good idea for threadless use cases.

Copy link
Member

@stylewarning stylewarning left a comment

Choose a reason for hiding this comment

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

I don't think this is a type-safe API.

@@ -38,7 +38,9 @@
:depends-on (#:coalton-compiler
#:coalton/hashtable-shim
#:trivial-garbage
#:alexandria)
#:alexandria
#+(or sb-thread threads openmcl-native-threads)
Copy link
Member

Choose a reason for hiding this comment

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

these should use the ASDF feature gates (ASDF has special syntax for this and your conditionals below)

;; Threads ;;
;;---------;;

(cl:defmacro spawn (cl:&body body)
Copy link
Member

Choose a reason for hiding this comment

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

My sense is to not include this, but open for debate.

library/threads.lisp Outdated Show resolved Hide resolved
library/threads.lisp Outdated Show resolved Hide resolved
library/threads.lisp Show resolved Hide resolved
;; Atomics ;;
;;---------;;

(coalton-toplevel
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 not sure the API below should have conditionals in it. Maybe atomic values shouldn't be included in the thread API for now.

library/threads.lisp Outdated Show resolved Hide resolved
@garlic0x1
Copy link
Contributor Author

I will make a lower-level LispThread type that Thread can be turned into for equality tests and have current-thread/all-threads return that. Should make it properly typesafe.

@garlic0x1 garlic0x1 marked this pull request as draft December 31, 2024 04:00
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.

2 participants