#65 Auto Multi-Core Support

closed-accepted
John Cerney
5
2011-05-12
2011-03-25
John Cerney
No

This patch adds support for splitting up numerical processing between multiple
parallel processor threads (or pthreads) using new functions "set_autopthread_targ"
and "set_autopthread_targ". This can improve processing performance (by greater than
2-4X in most cases) by taking advantage of multi-core and/or multi-processor machines.

See the new file Basic/Pod/ParallelCPU.pod for details.

This patch is against PDL 2.4.7 and is in git "format-patch" format

Discussion

  • Chris Marshall
    Chris Marshall
    2011-03-25

    Thanks for the patch, John. We're in feature freeze for PDL-2.4.8
    but I'm looking forward to applying this and trying it out post the
    2.4.8 release.

    --Chris

     
  • Chris Marshall
    Chris Marshall
    2011-04-30

    I notice that you are a registered PDL developer and so
    should have git access. Maybe it would make sense to
    apply the patch to git directly. The usual standard is to
    not push anything to git that does not build and pass
    tests on at least one platform. If there is possible instability,
    you could also create a branch for development until
    things are good for a merge back to the main trunk.

    --Chris

     
  • Chris Marshall
    Chris Marshall
    2011-04-30

    • assigned_to: nobody --> cerney
     
  • John Cerney
    John Cerney
    2011-05-03

    I can apply the patch directly if you would like. It builds and passes tests on Linux (several flavors of Ubuntu and Redhat enterprise).

    Currently the multi-threaded feature is Off by default so I think it is pretty safe to add to the main trunk. Is that what you had in mind, or should I still put it on a branch?

    -John

     
  • Chris Marshall
    Chris Marshall
    2011-05-03

    The basic motivation for putting things in a branch
    is to avoid destabilizing the truck so that developers
    pulling the latest can still have an expectation that
    new stuff won't break "make test".

    Changes that take a long time to implement or that
    affect low level (pervasive or Core) PDL features which
    make it time consuming the verify that things are OK
    need a branch.

    If the new feature is off by default, doesn't break your
    tests, apply away. :-)

    Be sure to e-mail the PDL list to announce the
    new feature and how to try/test. An addition to the
    test suite can allow for automated testing via the
    next CPAN Developers release. I'm happy to
    push a new developers release on request.

    Looking forward to trying it out... --Chris

     
  • David Mertens
    David Mertens
    2011-05-03

    The current patch, based on PDL 2.4.7, did not cleanly apply to the current git (01f90), so I had to modify it. The problem with the patch as submitted was that some useful comments were added to pdlcore.c.PL that defined the boundary of the barf section. Since git couldn't find those comments, it didn't apply the patch. I've modified the patch so that it now applies.

     
  • David Mertens
    David Mertens
    2011-05-03

    I found that this doesn't build when GSL is also built. Here are my steps to reproduce:

    1) Start from the latest git without the patch, with GSL libs installed.
    2) apply the patch
    2) make clean
    3) perl Makefile.PL
    4) make

    The build process ran fine until the GSL INTEG module, at which point it said:

    ---------------------------%<---------------------------
    cc -c -I/home/david/packages/PDL/Basic/Core -I/usr/include -D_REENTRANT -D_GNU_SOURCE -DDEBIAN -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -O2 -g -DVERSION=\"2.4.9_002\" -DXS_VERSION=\"2.4.9_002\" -fPIC "-I/usr/lib/perl/5.10/CORE" INTEG.c
    INTEG.xs: In function ‘XS_PDL_qng_meat’:
    INTEG.xs:14123: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘->’ token
    INTEG.xs:14123: error: expected expression before ‘->’ token
    INTEG.xs:14144: warning: assignment from incompatible pointer type
    INTEG.xs:14154: warning: assignment from incompatible pointer type
    INTEG.xs:14234: error: request for member ‘state’ in something not a structure or union
    INTEG.xs:14252: error: request for member ‘datatype’ in something not a structure or union
    INTEG.xs:14253: warning: passing argument 1 of ‘PDL->get_convertedpdl’ from incompatible pointer type
    INTEG.xs:14253: note: expected ‘struct pdl *’ but argument is of type ‘void (*)(const char *)’
    INTEG.xs:14253: warning: assignment from incompatible pointer type
    INTEG.xs:14274: warning: assignment from incompatible pointer type
    ...
    ---------------------------%<---------------------------

    The problem is that the GSL pp functions use a variable named 'warn' in their argument lists but the patch #defines the symbol 'warn' to be 'PDL->pdl_warn', which is no longer a valid variable name, obviously. That #definition gets inserted by PP into the XS code, and havoc ensues.

     
  • David Mertens
    David Mertens
    2011-05-04

    I grepped the source code for 'warn('. John's patch (and my modification) creates a C preprocessor macro that inserts PDL->pdl_warn in place of warn, and that impacts every compiled .pd file. It turns out that very few .pd files explicitly call warn(), so it may be better to simply change those instances by hand and avoid the preprocessor definition altogether.

     
  • John Cerney
    John Cerney
    2011-05-09

    The redefinition of warn to PDL->pdl_warn keeps perl's warn from being called in PP code when multi-threading. Calling perl's warn while multi-threading can cause a segfault. The new PDL->pdl_warn function saves warning messages while multi-threading and prints them out when the multi-threading is completed.

    Calling perl's warn inside of PP code appears to be fairly common. It shows up in the PDL package code, and in external libraries PP code (like some I have written). This is why the C preprocessor macro was added to define warn to PDL->pdl_warn. The change keeps existing PDL PP-code working with the new multi-threading support without any code changes.

    I think the simple fix for the GSL glue-code (gsl_integ.pd) is to undef the PDL->pdl_warn macro definition in this file, since it is not needed here anyway. The alternative is to have everywhere a warn was called changed to PDL->warn, which will cause segfaults when multi-threading if any warn to PDL->warn changes are missed. Basically I would prefer to work compilation issues like this when they come up rather than working strange segfault problems later.

    If there are no objections, I will make this fix to gsl_integ.pd in the patch and apply to the main trunk in git.

     
  • David Mertens
    David Mertens
    2011-05-09

    I see. Perl's warn has been a part of the API for some time, so we can't just remove it as I have proposed. That would be breaking backward compatibility. In that case, your solution is much better than mine.

    The only issue I have with the redefinition of warn is that it leads to nonsensical compiler errors when somebody uses the variable named 'warn', as was the case with the GSL INTEG bindings. The errors are caught at compile time, but I was only able to make sense of them because I knew they were due to the new patch. A note should be added to the PP docs that this is a reserved word so that if people run into problems, they will know why.

    That having been said, I think you can apply your patch since it will have no adverse effects. I'll test it when I get a chance.

     
  • John Cerney
    John Cerney
    2011-05-12

    • status: open --> open-accepted
     
  • John Cerney
    John Cerney
    2011-05-12

    Applied patch (with changes so gsl_integ.pdl compiles) to master in git.

     
  • John Cerney
    John Cerney
    2011-05-12

    • status: open-accepted --> closed-accepted