Server no place mode #995
Server no place mode #995
Conversation
zaneenders
force-pushed
the
zane-server-single-thread
branch
from
6c2583f
to
12f2b01
Compare
|
This PR does make the server more complex as it can now operate in two modes. But my goal with this PR and future PRs is to reduce the split code paths as well as introduce duplicate logic in Herbie. |
| (place-channel-get a)] | ||
| [else | ||
| (define result (hash-ref single-threaded-cache job-id #f)) | ||
| (if (false? result) (reverse (unbox (*timeline*))) result)])) |
There was a problem hiding this comment.
(if result result (reverse ...)) or just (and result (reverse ...)
There was a problem hiding this comment.
Also is this correct? If there is a result it looks like it'll save it.
Also wait why goes into single-threaded-cache? In fact, how can we ever call this method while the timeline is running? Why not just always return #f?
| @@ -79,56 +79,98 @@ | |||
| (rename-file-or-directory tmp-file data-file #t) | |||
| (copy-file (web-resource "report.html") html-file #t)) | |||
|
|
|||
| (define single-threaded-cache (make-hash)) | |||
There was a problem hiding this comment.
Why does this have to be a separate cache from whatever the manager uses?
There was a problem hiding this comment.
Well, I thought it had to be inside the manager for isolation reasons but main seems to work if I make completed-work a global.
| (define job-thread | ||
| (thread (λ () | ||
| (let loop ([seed #f]) | ||
| (match (thread-receive) | ||
| [(list work job-id semaphore) (single-thread-herbie work job-id semaphore)]) | ||
| (loop seed))))) | ||
| (define sema (make-semaphore)) | ||
| (set! job-running #t) | ||
| (thread-send job-thread (list command job-id sema)) | ||
| (semaphore-wait sema) | ||
| (set! job-running #f)]) ;; Block for job to finish |
There was a problem hiding this comment.
Why put this in a thread? Won't this thread keep running forever? Don't we only send it one job? Why have a thread at all?
There was a problem hiding this comment.
Also if there was no thread you wouldn't need a semaphor. And if you did that you wouldn't need the single-thread-herbie function which does very little on its own.
| (log "Job ~a, Qed up for program: ~a\n" job-id (test-name (herbie-command-test command))) | ||
| (place-channel-put manager (list 'start manager command job-id))] | ||
| [else | ||
| (log "Waiting for job ~a to finish.\n" job-id) |
| (define finished-result (place-channel-get a)) | ||
| (log "Done waiting for: ~a\n" job-id) | ||
| finished-result] | ||
| [else (hash-ref single-threaded-cache job-id #f)])) |
There was a problem hiding this comment.
I don't think this ever returns #f in multi-threaded-mode? Maybe just make it crash in single-threaded mode if you wait for a job you didn't start. (Also maybe in multi-threaded mode?)
| (set! manager-dead-event (place-dead-evt r)) | ||
| (set! manager r)) | ||
| (cond | ||
| [(> job-cap 1) |
There was a problem hiding this comment.
I think single-threaded mode should be separate from the concept of 1 thread. Maybe just make it a separate method or a global parameter or something?
| (define threads #f) | ||
| (define threads 1) |
There was a problem hiding this comment.
Again I think #f for no threads and 1 for one thread should be separate (because even in "1 thread" mode you in fact spawn a number of places, even though only one of them is a worker thread.)
This PR adds a "single thread" mode which the goal of is to have a mode that does not use places. Places can cause difficult debugging of lower-level core Herbie code so having a mode that does not add this indirection is a must before being able to use the server for other parts of Herbie like reports which generate the nightly.
In this mode, jobs are run when
start-jobis called and not whenwait-for-jobis called in order to support the demo. However, the demo doesn't stream the timeline and blocks until the job is completed.