[Sbcl-devel] Merge request for windows threads

[Kalyanov Dmitry]

I'd like to consider the possibility of merging Windows threads support
into SBCL main tree. I've put up page with information about threads
support at http://dmitryvk.github.com/sbcl-win32-threads/ (including
links to the code).

How should I proceed? Should I chop this patch to smaller pieces or can
it be applied as one big patch? Safepoints may also be useful for
threading on other platforms - should safepoints be ported first or it's
better to refactored them later?

I'd like to hear any advice.

------------------------------------------------------------------------------
Learn how Oracle Real Application Clusters (RAC) One Node allows customers
to consolidate database storage, standardize their database environment, and,
should the need arise, upgrade to a full multi-node Oracle RAC database
without downtime or disruption
http://p.sf.net/sfu/oracle-sfdevnl
_______________________________________________
Sbcl-devel mailing list
Sbcl-...@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sbcl-devel

[Alastair Bridgewater]

I have a few questions about this this branch that I'd like to see
answered prior to merging.

The enter/leave region functions seem... ungraceful, somehow. There is
a lot of repetitive code that could be papered over via a MACROLET,
the function names are ugly in context, and the /no-fixup bit could
just as easily be an optional or keyword parameter. But there's one
further possibility: Have you considered using an instruction-macro,
so that the call site becomes something on the order of (inst
enter-safe-region :no-fixup) ?

In the definition for +win32-tib-arbitrary-field-offset+, there is no
commentary as to what this offset corresponds to, it ends up just
being a magic number. Can this reasonably be added, along with a
justification for why hacking up an arbitrary field in the TIB is
"okay"? (I see that this is explained elsewhere, but there's nothing
to link the definition to the explanation... or vice versa.)

Why emulate pthreads and posix signal handling instead of doing
something more "windows-like"?

Why even have definitions for signals other than SIG_STOP_FOR_GC and
SIGPIPE (for interrupt-thread), and possibly SIGINT (for use by
SetConsoleCtrlHandler() handlers)?

Those are the major questions, to my mind, at least.

On Sun, Dec 26, 2010 at 2:48 PM, Kalyanov Dmitry
<kalyano...@gmail.com> wrote:
> I'd like to consider the possibility of merging Windows threads support
> into SBCL main tree. I've put up page with information about threads
> support at http://dmitryvk.github.com/sbcl-win32-threads/ (including
> links to the code).
>
> How should I proceed? Should I chop this patch to smaller pieces or can
> it be applied as one big patch? Safepoints may also be useful for
> threading on other platforms - should safepoints be ported first or it's
> better to refactored them later?

Smaller pieces? Definitely! Consider the linux/ppc threading changes:
Some 40 separate commits in the 1.0.41.x series (essentially, almost
all of the commits I made for that release were related to ppc
threading).

Integrating safepoints first? Yes, probably: It's plausibly useful on
other platforms (perhaps making it work on linux would be a good
start), and constitutes a "smaller piece", although it should probably
be split up into more than one commit as well.

> I'd like to hear any advice.

-- Alastair Bridgewater

[Roman Marynchak]

It would be nice to remove other magic numbers from the patch as well. I have seen several "4" hardcoded in the patch (mostly in SB-VM part of it). It looks like the 32-bit pointer size, but this is not a justification. 

In general (not in this patch), magic numbers are quite common in SBCL's SB-VM code, and this is a problem. When I look at this code from call.lisp, I wonder why don't we name the things properly?

(+ (if named 5 0)

      (if variable 19 1)

        (if (eq return :tail) 0 10)

            15

            (if (eq return :unknown) 25 0))

Regards,

Roman

2010/12/30 Alastair Bridgewater <alastair.b...@gmail.com>

[Paul Khuong]

On 2010-12-30, at 8:16 AM, Roman Marynchak wrote:

> It would be nice to remove other magic numbers from the patch as well. I
> have seen several "4" hardcoded in the patch (mostly in SB-VM part of it).
> It looks like the 32-bit pointer size, but this is not a justification.
>
> In general (not in this patch), magic numbers are quite common in SBCL's
> SB-VM code, and this is a problem. When I look at this code from call.lisp,
> I wonder why don't we name the things properly?
>
> (+ (if named 5 0)
> (if variable 19 1)
> (if (eq return :tail) 0 10)
> 15
> (if (eq return :unknown) 25 0))

I don't even have to look at the source to tell that this is computing a VOP's cost. Those are currently almost arbitrary, except for the total order between equally applicable VOPs. Frankly, I have no idea how one would give a name to these value, except for the obvious name from the condition they depend on.

Paul Khuong

[Pascal J. Bourguignon]

Roman Marynchak <roman.m...@gmail.com> writes:

> It would be nice to remove other magic numbers from the patch as well. I
> have seen several "4" hardcoded in the patch (mostly in SB-VM part of
> it). It looks like the 32-bit pointer size, but this is not a
> justification. 
>
> In general (not in this patch), magic numbers are quite common in SBCL's
> SB-VM code, and this is a problem. When I look at this code from
> call.lisp, I wonder why don't we name the things properly?
>
> (+ (if named 5 0)
>       (if variable 19 1)
>         (if (eq return :tail) 0 10)
>             15
>             (if (eq return :unknown) 25 0))

Without mentionning the indentation...

(+ (if named 5 0)
(if variable 19 1)
(if (eq return :tail) 0 10)
15
(if (eq return :unknown) 25 0))

and unless return is a symbol-macro with side effects,

(+ 15

(if named 5 0)
(if variable 19 1)

(case return
((:tail) 0)
((:unknown) 35)
(otherwise 10)))

--
__Pascal Bourguignon__ http://www.informatimago.com/
A bad day in () is better than a good day in {}.