From: Daniel B. <da...@te...> - 2003-03-31 00:15:23
|
Having got my thread-supporting checkout area to build a single-threaded x86 SBCL with no obvious regressions, I think this is a safe time to announce that the thread merge is imminent. The patch is ~3700 lines long, of which about 2/3 are runtime changes. This makes for 130k of patch file, so I won't post it to the list. You can however get it from http://ww.telent.net/sbcl-thread-merge.txt I intend to apply this some time on Monday evening (UK time == GMT+1), so if you'd like to comment on it, before then would be good. If it's going in in approximately its current form I'd rather get it committed as early in the month as practical, to allow more people to beat on it. -dan -- http://www.cliki.net/ - Link farm for free CL-on-Unix resources |
From: Christophe R. <cs...@ca...> - 2003-03-31 10:21:11
|
Daniel Barlow <da...@te...> writes: > I intend to apply this some time on Monday evening (UK time == GMT+1), > so if you'd like to comment on it, before then would be good. If it's > going in in approximately its current form I'd rather get it committed > as early in the month as practical, to allow more people to beat on it. Some quick linear observations: +#$gnumake clean || exit 1 +#$gnumake depend || exit 1 ... :-) - (load-symbol-value catch *current-catch-block*) + (load-tl-symbol-value catch *current-catch-block*) Is it possible to have it be LOAD-THREAD-LOCAL-SYMBOL-VALUE? I know that's wordy and rightward-drifty, but for future (and indeed present) maintainers' sake... #!+x86 +(defun nth-interrupt-context (n) Is this definitely an x86-only operation? If it's only used inside the x86 version of FIND-CURRENT-FRAME, maybe it could be a FLET inside that? +;;; FIXME II: this definiton is supposed to be in effect when we're +;;; doing make-host-2 and +sb-thread. If you can mre clearly express +;;; the conditionals here without defining this macro twice, please +;;; clue me in + +#!+sb-thread #-sb-xc-host Do you want a version of ATOMIC-INCF defined on the host? It looks to me as though none such would be defined with #!+sb-thread, though one _is_ defined in #!-sb-thread. I can see that it looks like a wordy problem; I think the sbcl-idiomatic solution is to define portable host versions in a file called src/code/cross-sysmacs.lisp, so (defmacro atomic-incf (&rest args) `(incf ,@args)) and the target versions in src/code/target-sysmacs.lisp (defmacro atomic-incf (...) #!+sb-thread ... #!-sb-thread ...) Also, you're not going to like this, but since it only currently works on symbols, consider naming it ATOMIC-INCF/SYMBOL or somesuch? [ aside here: it would be nice to support atomic increments of generalized memory references, where this is possible, ideally transparently -- so something like (locally (declare atomic) ; implies (SAFETY 0) (incf (car x))) would just work. However, this looks like a complicated problem -- the only way I could see of doing this even vaguely right involved either a codewalk of the body, or compiler surgery; we only have a hope on symbols with the current layout of the compiler because setting a symbol's value has a special compiler node type: a SET, not a regular combination of something like %RPLACA. ] #-sb-xc-host +#!-sb-thread +(defmacro with-recursive-lock ((mutex) &body body) + `(progn ,@body)) + +#-sb-xc-host +#!+sb-thread Again, doesn't this kind of thing belong in target-{uni,}thread? There were some other things that made me feel slightly queasy, like +#define BINDING_STACK_SIZE (1024*1024) /* chosen at random */ +#define THREAD_CONTROL_STACK_SIZE (2*1024*1024) /* wired elsewhere-watch out */ but absent data on whether it works (threaded or not) on non-x86/Linux, which we're probably not going to get pre-merge-on-HEAD anyway, I'd say go for it, and good stuff! Cheers, Christophe -- http://www-jcsu.jesus.cam.ac.uk/~csr21/ +44 1223 510 299/+44 7729 383 757 (set-pprint-dispatch 'number (lambda (s o) (declare (special b)) (format s b))) (defvar b "~&Just another Lisp hacker~%") (pprint #36rJesusCollegeCambridge) |
From: Daniel B. <da...@te...> - 2003-03-31 13:26:41
|
Christophe Rhodes <cs...@ca...> writes: > Some quick linear observations: Thanks. > +#$gnumake clean || exit 1 > +#$gnumake depend || exit 1 Ahem. Yes, I usually spot this before committing if rarely before difffing > - (load-symbol-value catch *current-catch-block*) > + (load-tl-symbol-value catch *current-catch-block*) > > Is it possible to have it be LOAD-THREAD-LOCAL-SYMBOL-VALUE? I know > that's wordy and rightward-drifty, but for future (and indeed present) > maintainers' sake... Hoist on what looks uncannily like my own petard here. Should actually be LOAD-SYMBOL-THREAD-LOCAL-VALUE really, I suppose. > #!+x86 > +(defun nth-interrupt-context (n) > > Is this definitely an x86-only operation? If it's only used inside > the x86 version of FIND-CURRENT-FRAME, maybe it could be a FLET inside > that? There will need to be a non-x86 version before non-x86 works again (lisp_interrupt_contexts is now defunct). That said, I suspect that the current definition is actually portable, as all the magic happens in current-thread-offset-sap. > Do you want a version of ATOMIC-INCF defined on the host? It looks to I want the complicated definition on the target when threads are in effect, and the null version (above the comment block) on the host and on the unithread target. > me as though none such would be defined with #!+sb-thread, though one > _is_ defined in #!-sb-thread. I can see that it looks like a wordy > problem; I think the sbcl-idiomatic solution is to define portable > host versions in a file called src/code/cross-sysmacs.lisp, so > (defmacro atomic-incf (&rest args) > `(incf ,@args)) > and the target versions in src/code/target-sysmacs.lisp > (defmacro atomic-incf (...) > #!+sb-thread ... > #!-sb-thread ...) Sounds reasonable, if slightly OAOOM (we're duplicating the trivial definition in cross-compiler and target-sans-threads, but hey, it is after all trivial). > Also, you're not going to like this, but since it only currently works > on symbols, consider naming it ATOMIC-INCF/SYMBOL or somesuch? It's only used in about three places, I think, (unlike, say, load-tl-symbol-value ...) so renaming should be easy > There were some other things that made me feel slightly queasy, like > > +#define BINDING_STACK_SIZE (1024*1024) /* chosen at random */ > +#define THREAD_CONTROL_STACK_SIZE (2*1024*1024) /* wired elsewhere-watch out */ I think the latter of these is going to be disappearing in the fairly near future: to support threads with different stack sizes (which we'll sooner or later want to do) we'll have to keep a size or end pointer in the thread structure somewhere. The binding stack size was chosen at random, so if the comment makes you feel queasy, that's an accurate reflection of the design. It's big enough to rebuild the system: people who do other things with SBCL as well are invited to complain if they can blow the binding stack -dan -- http://www.cliki.net/ - Link farm for free CL-on-Unix resources |