Closed appleby closed 4 years ago
Added some basic concurrency tests to sanity check locking around persistent QVMs. Found and fixed one bug related to calling uuid:make-v4-uuid
from multiple threads.
I also had some concurrent http tests, but removed them at the last minute. It turns out its pretty hard to tickle concurrency bugs through hunchentoot for various reasons. I might add some concurrent http tests back if I can figure out how to do it in a non-fiddly way.
I'm not plugged into this effort enough to give it a proper review w/o a lot of back-reading, but my passing appraisal is that it looks very good!
I'm going to continue to use this branch & PR for ongoing work, so I've slapped a work-in-progress label on it for now.
The latest commits add a SAFETY-HASH package with both a generic BORDEAUX-THREADS-based implementation and an SBCL-specific implementation that uses SBCL's :SYNCHRONIZED and WITH-LOCKED-HASH-TABLE extensions.
The following naive insert/read/delete test indicates the SBCL-specific implementation is ~2x faster on my laptop, even though (based on a cursory examination) SBCL's implementation of these extensions appears to be doing something similar to the generic BORDEAUX-THREADS implementation: namely acquiring and releasing a recursive per-hash-table lock around the standard hash-table getter/setter/remover functions (see e.g. PICK-TABLE-METHODS in sbcl/src/code/target-hash-table.lisp). Likewise, SB-EXT:WITH-LOCKED-HASH-TABLE doesn't appear to be doing anything fancy... Perhaps the key difference is that SBCL pushes the locking down as far as possible, whereas the generic BORDEAUX-THREADS impl grabs the lock higher up the callstack. Or maybe it's down to the difference between SB-THREAD::CALL-WITH-RECURSIVE-LOCK vs SB-THREAD::CALL-WITH-RECURSIVE-SYSTEM-LOCK, which doesn't do any interrupt twiddling. 'Tis a mystery.
This is a pretty lazy benchmark, but the speedup was convincing enough that I didn't feel the need to do more elaborate benchmarking.
(require :sb-sprof)
(ql:quickload :bordeaux-threads)
(load (compile-file #P"/Users/mappleby/src/repos/rigetti/qvm/app-ng/src/safety-hash/package.lisp"))
;;(load (compile-file #P"/Users/mappleby/src/repos/rigetti/qvm/app-ng/src/safety-hash/impl-bordeaux-threads.lisp"))
(load (compile-file #P"/Users/mappleby/src/repos/rigetti/qvm/app-ng/src/safety-hash/impl-sbcl.lisp"))
(defun safety-hash-test (n)
(let ((h (safety-hash:make-safety-hash)))
(dotimes (i (expt 2 n) nil)
(setf (safety-hash:gethash i h) 42)
(safety-hash:gethash i h)
(safety-hash:remhash i h))))
(sb-sprof:with-profiling (:report :flat)
(safety-hash-test 26))
Here are the top-20 results of running the above test on the generic impl-bordeaux-threads.lisp
.
Number of samples: 2144
Sample interval: 0.01 seconds
Total sampling time: 21.439999 seconds
Number of cycles: 0
Sampled threads:
#<SB-THREAD:THREAD "repl-thread" RUNNING {1002191BC3}>
Self Total Cumul
Nr Count % Count % Count % Calls Function
------------------------------------------------------------------------
1 515 24.0 1164 54.3 515 24.0 - (FLET "WITHOUT-INTERRUPTS-BODY-29" :IN SB-THREAD::CALL-WITH-RECURSIVE-LOCK)
2 462 21.5 1958 91.3 977 45.6 - QVM-APP-NG.SAFETY-HASH:CALL-WITH-LOCKED-SAFETY-HASH
3 384 17.9 384 17.9 1361 63.5 - SB-UNIX::WITH-DEFERRABLE-SIGNALS-UNBLOCKED
4 205 9.6 205 9.6 1566 73.0 - (FLET "CLEANUP-FUN-41" :IN SB-THREAD::CALL-WITH-RECURSIVE-LOCK)
5 116 5.4 1487 69.4 1682 78.5 - SB-THREAD::CALL-WITH-RECURSIVE-LOCK
6 49 2.3 49 2.3 1731 80.7 - (SETF QVM-APP-NG.SAFETY-HASH:GETHASH)
7 48 2.2 58 2.7 1779 83.0 - SB-IMPL::REMHASH/EQL
8 42 2.0 42 2.0 1821 84.9 - QVM-APP-NG.SAFETY-HASH:GETHASH
9 34 1.6 2132 99.4 1855 86.5 - SAFETY-HASH-TEST
10 34 1.6 34 1.6 1889 88.1 - QVM-APP-NG.SAFETY-HASH:REMHASH
11 32 1.5 32 1.5 1921 89.6 - (FLET SB-THREAD::WITH-RECURSIVE-LOCK-THUNK :IN QVM-APP-NG.SAFETY-HASH:CALL-WITH-LOCKED-SAFETY-HASH)
12 31 1.4 50 2.3 1952 91.0 - SB-IMPL::PUTHASH/EQL
13 27 1.3 27 1.3 1979 92.3 - (LAMBDA (HASH-TABLE) :IN QVM-APP-NG.SAFETY-HASH:GETHASH)
14 22 1.0 22 1.0 2001 93.3 - SB-VM::ALLOC-TRAMP
15 20 0.9 20 0.9 2021 94.3 - (LAMBDA (HASH-TABLE) :IN QVM-APP-NG.SAFETY-HASH:GETHASH)
16 19 0.9 19 0.9 2040 95.1 - (FLET SB-IMPL::INSERT-AT :IN SB-IMPL::GET-PUTHASH-DEFINITIONS)
17 17 0.8 17 0.8 2057 95.9 - SB-IMPL::GETHASH/EQL
18 16 0.7 67 3.1 2073 96.7 - SB-KERNEL:%PUTHASH
19 15 0.7 15 0.7 2088 97.4 - (LAMBDA (HASH-TABLE) :IN QVM-APP-NG.SAFETY-HASH:REMHASH)
20 12 0.6 12 0.6 2100 97.9 - REMHASH
...[snip]...
------------------------------------------------------------------------
7 0.3 elsewhere
And here are the results of running the same test with impl-sbcl.lisp
.
Number of samples: 986
Sample interval: 0.01 seconds
Total sampling time: 9.86 seconds
Number of cycles: 0
Sampled threads:
#<SB-THREAD:THREAD "repl-thread" RUNNING {10021B1BC3}>
Self Total Cumul
Nr Count % Count % Count % Calls Function
------------------------------------------------------------------------
1 776 78.7 776 78.7 776 78.7 - (FLET "CLEANUP-FUN-69" :IN SB-THREAD::CALL-WITH-RECURSIVE-SYSTEM-LOCK)
2 65 6.6 863 87.5 841 85.3 - SB-THREAD::CALL-WITH-RECURSIVE-SYSTEM-LOCK
3 19 1.9 976 99.0 860 87.2 - SAFETY-HASH-TEST
4 19 1.9 19 1.9 879 89.1 - (FLET "WITHOUT-INTERRUPTS-BODY-57" :IN SB-THREAD::CALL-WITH-RECURSIVE-SYSTEM-LOCK)
5 15 1.5 325 33.0 894 90.7 - SB-KERNEL:%PUTHASH
6 12 1.2 313 31.7 906 91.9 - SB-IMPL::PUTHASH/EQL/LOCK
7 12 1.2 12 1.2 918 93.1 - SB-VM::ALLOC-TRAMP
8 11 1.1 274 27.8 929 94.2 - SB-IMPL::GETHASH/EQL/LOCK
9 11 1.1 11 1.1 940 95.3 - QVM-APP-NG.SAFETY-HASH:GETHASH
10 10 1.0 315 31.9 950 96.3 - SB-IMPL::REMHASH/EQL/LOCK
11 9 0.9 9 0.9 959 97.3 - REMHASH
12 6 0.6 6 0.6 965 97.9 - (SETF QVM-APP-NG.SAFETY-HASH:GETHASH)
13 5 0.5 5 0.5 970 98.4 - SB-IMPL::GETHASH3
14 4 0.4 4 0.4 974 98.8 - QVM-APP-NG.SAFETY-HASH:REMHASH
15 0 0.0 979 99.3 974 98.8 - "Unknown component: #x2283B2C0"
16 0 0.0 979 99.3 974 98.8 - SWANK::EVAL-REGION
17 0 0.0 979 99.3 974 98.8 - (LAMBDA NIL :IN SWANK-REPL::REPL-EVAL)
18 0 0.0 979 99.3 974 98.8 - SWANK-REPL::TRACK-PACKAGE
19 0 0.0 979 99.3 974 98.8 - SWANK::CALL-WITH-RETRY-RESTART
20 0 0.0 979 99.3 974 98.8 - SWANK::CALL-WITH-BUFFER-SYNTAX
...[snip]...
------------------------------------------------------------------------
5 0.5 elsewhere
Updates #186
To celebrate a century of commits (The Big Cien), this latest round includes basic job management endpoints, create-job
, job-info
, delete-job
, etc.
This also includes the latest commits from #180, squashed down to a single commit. I've temporarily structured things to avoid merge conflicts. Once the other PR lands, there will be some light refactoring to integrate the new job-related RPC endpoints with the stuff in #180.
I'm going to continue to use this branch & PR for ongoing work, so I've slapped a work-in-progress label on it for now.
Honestly forgot I had marked this WIP. There are still some odds and ends to be done, but this is ready for review for anyone who's idea of halloween fun includes ghastly qvm hacks.
General status update on where this PR is at: what is implemented so far are the endpoints for managing persistent qvms + async jobs + a generic run-program
endpoint + a handful of minor bookkeeping endpoints like version
, qvm-memory-estimate
, etc.
There is still a fair bit of work to do to get this to a point where it could replace the existing qvm-app and be considered "production ready". At a minimum that work would include:
Fleshing out API endpoints to have equivalents for all existing qvm-app endpoints (multishot, expectation, wavefunction, etc.)
General work towards qvm-app feature parity, including support for shared-memory allocation, noise models beyond pauli noise, benchmarking, command line mode, etc.
Resource monitoring and enforcement (limits on memory/qubits/async jobs/handler timeouts, etc.)
Improved logging (very little being logged right now)
Some clear plan for how to integrate this on the pyquil side. Currently I have a draft-mode pyquil PR that adds a new PersistentQVM
class which acts like a "grab-bag of methods" for interacting with the new qvm-ng API, but it sits off to the side and is not well integrated with the existing pyquil class structure of QAM
s and QVM
s and QuantumComputers
.
... and probably other stuff I'm forgetting.
On the other hand, the only change to the existing qvm code included in this PR is the addition of the WAIT-FUNCTION
slot to the CLASSICAL-MEMORY-MIXIN
class, which is to say I think these changes could be safely merged with low risk of breaking the existing qvm-app.
diff --git a/src/classical-memory-mixin.lisp b/src/classical-memory-mixin.lisp
index f144b1a..b722a82 100644
--- a/src/classical-memory-mixin.lisp
+++ b/src/classical-memory-mixin.lisp
@@ -4,6 +4,10 @@
(in-package #:qvm)
+(defun warning-wait-function (qvm)
+ (declare (ignore qvm))
+ (warn "WAIT executed. Nothing to wait on."))
+
;;; The non-quantum parts of a QVM (the program, the pc, and the
;;; classical memory subsystem) don't really change from QVM to QVM,
;;; so that state is stored in an abstract mixin class.
@@ -22,11 +26,15 @@
(program :accessor program
:initform #()
:documentation "The program to be executed.")
+ (wait-function :reader wait-function
+ :initarg :wait-function
+ :documentation "A unary function taking a QVM and implementing Quil's WAIT logic. (The default does nothing and just warns.)")
(gate-definitions :accessor gate-definitions
:initarg :gate-definitions
:documentation "A table mapping gate names to their GATE-instance definition."))
(:default-initargs
:classical-memory-subsystem (make-instance 'qvm:classical-memory-subsystem)
+ :wait-function 'warning-wait-function
;; XXX FIXME: To be superseded by some notion of environments.
:gate-definitions (copy-hash-table quil::**default-gate-definitions**))
(:metaclass abstract-class)
diff --git a/src/transition-classical-instructions.lisp b/src/transition-classical-instructions.lisp
index 7f4d687..2c5e01e 100644
--- a/src/transition-classical-instructions.lisp
+++ b/src/transition-classical-instructions.lisp
@@ -373,3 +373,11 @@
quil:classical-greater-equal-bit/octet/immediate
quil:classical-greater-equal-bit/integer/immediate
quil:classical-greater-equal-bit/real/immediate)
+
+;;; WAIT!
+
+(defmethod transition ((qvm classical-memory-mixin) (instr quil:wait))
+ (declare (ignore instr))
+ (funcall (wait-function qvm) qvm)
+ (incf (pc qvm))
+ qvm)
diff --git a/src/transition.lisp b/src/transition.lisp
index 750f856..efdcf5d 100644
--- a/src/transition.lisp
+++ b/src/transition.lisp
@@ -94,13 +94,6 @@ Return just the resulting (possibly modified) QVM after executing INSTR. (Histor
(setf (pc qvm) (1+ (pc measured-qvm)))
qvm)))
-(defmethod transition ((qvm classical-memory-mixin) (instr quil:wait))
- (declare (ignore instr))
- (when *transition-verbose*
- (warn "WAIT executed. Nothing to wait on."))
- (incf (pc qvm))
- qvm)
-
;;;;;;;;;;;;;;;;;;;; JUMP, JUMP-WHEN, JUMP-UNLESS ;;;;;;;;;;;;;;;;;;;;
Using it in production can be another matter—lots of QVM features don’t appear in production, in the sense that they aren’t part of the typical execution path & instead require deliberate intervention to turn on and utilize. Certain select groups still make use of them, & that’s the sort of usage I think would provide better testing / feedback than another PR read-through.
welp, here goes nothin
This PR includes implementations of the following API methods:
No support for allocation types or noise models.Partial support for allocation types (native and foreign) and noise models (currently only Pauli channel noise). No reconfiguration of persistent QVMs.Only simulation-method (pure-state or full-density-matrix) and number of qubits are specified at creation time.The
run-program
API call can act either on a persistent QVM (if passed a qvm-token parameter) or else on an ephemeral QVM (if passed simulation and allocation methods). It's interface is similar to the currentmultishot
call, but without multiple trials. Asingleshot
, if you will.API handlers are defined via a macro
DEFINE-RPC-HANDLER
which is similar in spirit to hunchentoot'sDEFINE-EASY-HANDLER
, but rather than dispatching based on the URI path, it dispatches based on the "type" field in the JSON request body, similar to the way the existing API dispatch works. Named parameters in the lambda-list ofDEFINE-RPC-HANDLER
are also populated from the JSON request body, as opposed to being initialized from GET/POST parameters as is the case forDEFINE-EASY-HANDLER
.This is still undercooked and woefully underdocumented, but posting now for exposure.TODO
Resolves #39