patternMinor
Message queues in Common Lisp
Viewed 0 times
messagelispcommonqueues
Problem
General advice for a change. This is an implementation of message queues that I'm going to use for some work on an actors model library.
```
(defclass message-queue ()
((messages :accessor messages :initarg :messages :initform nil)
(last-cons :accessor last-cons :initarg :last-cons :initform nil
:documentation "Cached end of the list")
(len :accessor len :initarg :len :initform 0
:documentation "Cached message queue length. Modified by enqueue and dequeue")
(lock :initform (bt:make-lock) :accessor lock
:documentation "Lock for this message queue")
(max-len :accessor max-len :initarg :max-len :initform nil
:documentation "If present, queue maintains at most this many elements")
(flag :initform (bt:make-condition-variable) :accessor flag
:documentation "Condition variable used to notify that a message was enqueued")))
(defun make-queue (&optional max-len)
(make-instance 'message-queue :max-len max-len))
(defmethod full-p ((queue sized-queue))
(with-slots (len max-len)
(and max-len (>= len max-len))))
(defmethod empty-p ((queue message-queue))
(= (len queue) 0))
(defmethod enqueue (object (queue message-queue))
"Adds an element to the back of the given queue in a thread-safe way."
(with-slots (lock messages max-len len flag last-cons) queue
(with-lock-held (lock)
(let ((o (list object)))
(cond ((empty-p queue)
(setf messages o
last-cons messages
len 1))
((full-p queue)
(pop messages)
(setf (cdr last-cons) o
last-cons o))
(t (setf (cdr last-cons) o
last-cons o)
(incf len)))))
(condition-notify flag)
messages))
(defmethod dequeue ((queue message-queue) &optional (timeout 0))
"Pops a message from the given queue in a thread-safe way.
If the target queue is empty, blocks until a message arrives.
If timeout is not zero, errors after timeout."
(with-slots (messages lock fla
```
(defclass message-queue ()
((messages :accessor messages :initarg :messages :initform nil)
(last-cons :accessor last-cons :initarg :last-cons :initform nil
:documentation "Cached end of the list")
(len :accessor len :initarg :len :initform 0
:documentation "Cached message queue length. Modified by enqueue and dequeue")
(lock :initform (bt:make-lock) :accessor lock
:documentation "Lock for this message queue")
(max-len :accessor max-len :initarg :max-len :initform nil
:documentation "If present, queue maintains at most this many elements")
(flag :initform (bt:make-condition-variable) :accessor flag
:documentation "Condition variable used to notify that a message was enqueued")))
(defun make-queue (&optional max-len)
(make-instance 'message-queue :max-len max-len))
(defmethod full-p ((queue sized-queue))
(with-slots (len max-len)
(and max-len (>= len max-len))))
(defmethod empty-p ((queue message-queue))
(= (len queue) 0))
(defmethod enqueue (object (queue message-queue))
"Adds an element to the back of the given queue in a thread-safe way."
(with-slots (lock messages max-len len flag last-cons) queue
(with-lock-held (lock)
(let ((o (list object)))
(cond ((empty-p queue)
(setf messages o
last-cons messages
len 1))
((full-p queue)
(pop messages)
(setf (cdr last-cons) o
last-cons o))
(t (setf (cdr last-cons) o
last-cons o)
(incf len)))))
(condition-notify flag)
messages))
(defmethod dequeue ((queue message-queue) &optional (timeout 0))
"Pops a message from the given queue in a thread-safe way.
If the target queue is empty, blocks until a message arrives.
If timeout is not zero, errors after timeout."
(with-slots (messages lock fla
Solution
I noticed you defined full read/write accessors on all the slots on your message queue class, yet you bypass all of that and just use with-slots. Are you planning on exporting those accessors as part of the API? It seems like you wouldn't want to encourage people to monkey with those slot values, so probably not. You could also drop :initarg from everything but max-len.
Context
StackExchange Code Review Q#14036, answer score: 2
Revisions (0)
No revisions yet.