[Bug libstdc++/34797] New: [parallel mode] Settings are separated for each compilation unit

[Bug libstdc++/34797] New: [parallel mode] Settings are separated for each compilation unit

[singler at gcc dot gnu dot org]

In settings.h, the Settings class (containing only static variables) is
encapsulated in an anonymous namespace. This makes the linker think of a
distinct class for each compilation unit. As as result, settings changed in one
.cpp file won't affect code from another .cpp file, although they should have
global effect.


-- 
           Summary: [parallel mode] Settings are separated for each
                    compilation unit
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: libstdc++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: singler at gcc dot gnu dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34797

[Bug libstdc++/34797] [parallel mode] Settings are separated for each compilation unit

[singler at gcc dot gnu dot org]

------- Comment #1 from singler at gcc dot gnu dot org  2008-01-15 13:15 -------
First of all, we should get rid of these many static variables in the Settings
class, and replace them by usual member variables. Then, there should be one
static/global instance of this Settings class.
The question remains how to concretely implement this static variable.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34797

[Bug libstdc++/34797] [parallel mode] Settings are separated for each compilation unit

[singler at gcc dot gnu dot org]

------- Comment #2 from singler at gcc dot gnu dot org  2008-01-15 13:20 -------
There are two general options to fix this bug:
1. Introduce a global variable, to be compiled into libstdc++.a and
libstdc++.so.
2. Do the "template trick", i. e. pseudo-parametrize Settings as
template class, to leave it to the compiler to merge the instantiations.
The class would always have to instantiated with the same template arguments,
but that could be hidden by a typedef.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34797

[Bug libstdc++/34797] [parallel mode] Settings are separated for each compilation unit

[bkoz at gcc dot gnu dot org]

------- Comment #3 from bkoz at gcc dot gnu dot org  2008-01-17 17:20 -------

I'd actually worked up a solution that takes the mt_allocator.h approach of a
settings datum, with public get/set methods. Thus, the default datum is private
to the libstdc++.a/.so binary. 

This is the eventual solution, I think, but maybe not the best temporary one. 

I'd not finished it off, as there was some question about forward compat. I'll
attach include/parallel/settings.h to this bug report.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34797

[Bug libstdc++/34797] [parallel mode] Settings are separated for each compilation unit

[bkoz at gcc dot gnu dot org]

------- Comment #4 from bkoz at gcc dot gnu dot org  2008-01-17 17:21 -------
Created an attachment (id=14958)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=14958&action=view)
__gnu_parallel:Settings as a datum


Here's just the settings code, but not the entire patch to implement this. 

I did this just to clarify to Johannes what I was talking/muttering about.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34797

[Bug libstdc++/34797] [parallel mode] Settings are separated for each compilation unit

[bkoz at gcc dot gnu dot org]

------- Comment #5 from bkoz at gcc dot gnu dot org  2008-01-18 23:22 -------

Brain dump into this: reasons to move to datum/mt_allocator type approach:

It tries to be the minimal change, keeping all your existing data. (With the
exception of making a tristate variable for the force_parallel/force_sequential
stuff.) What it does is make __gnu_parallel::Settings into a datum that has two
static accessors,  Settings::get and Settings::set methods.

Then, when one needs a config parameter, they do

const __gnu_parallel::Settings& s = __gnu_parallel::Settings::get();

And then access the settings data via

s.partition_minimal_n;

etc.

Let me know what you think about this. Hopefully it adds something more
concrete to our discussions.

I didn't do the rest of the patch, which is to fix up the current accesses. I
wanted to make sure we are on the same wavelength here first, design-wise.

The advantage of doing things in this general way is:

1) Actual settings are private, internal to the library binary.
2) Only two exports are needed: the get and set methods
3) Can add (but not subtract) from settings datum later on.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34797

[Bug libstdc++/34797] [parallel mode] Settings are separated for each compilation unit

[bkoz at gcc dot gnu dot org]

------- Comment #6 from bkoz at gcc dot gnu dot org  2008-02-18 00:00 -------
Subject: Bug 34797

Author: bkoz
Date: Mon Feb 18 00:00:00 2008
New Revision: 132383

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=132383
Log:
2008-02-17  Benjamin Kosnik  <bkoz@redhat.com>

        PR libstdc++/34797
        * include/parallel/settings.h (_Settings): Reconstruct Settings class
        here, uglify, remove anonymous namespace and static
        members. Convert to datum.      
        * include/parallel/types.h: Move Settings:: enumerations here, uglify.
        * src/parallel_settings.cc: New, definition for _Settings member
        functions.      
        * include/parallel/multiway_merge.h: Same.
        * include/parallel/for_each.h: Same.
        * include/parallel/workstealing.h: Same.
        * include/parallel/base.h: Same.
        * include/parallel/numeric
        * include/parallel/features.h: Same.
        * include/parallel/quicksort.h: Same.
        * include/parallel/equally_split.h: Same.
        * include/parallel/algorithmfwd.h: Same.
        * include/parallel/omp_loop_static.h: Same.
        * include/parallel/random_shuffle.h: Same.
        * include/parallel/balanced_quicksort.h: Same.
        * include/parallel/tags.h: Same.
        * include/parallel/multiway_mergesort.h: Same.
        * include/parallel/numericfwd.h: Same.
        * include/parallel/partition.h: Same.
        * include/parallel/partial_sum.h: Same.
        * include/parallel/find.h: Same.
        * include/parallel/algo.h: Same.
        * include/parallel/omp_loop.h: Same.
        * include/parallel/sort.h: Same.

        * src/Makefile.am (parallel_sources): Add parallel_settings.cc.
        * src/Makefile.in: Regenerate.

        * config/abi/pre/gnu.ver: Export _Settings::get and _Settings::set.


Added:
    trunk/libstdc++-v3/src/parallel_settings.cc
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/config/abi/pre/gnu.ver
    trunk/libstdc++-v3/include/parallel/algo.h
    trunk/libstdc++-v3/include/parallel/algorithmfwd.h
    trunk/libstdc++-v3/include/parallel/balanced_quicksort.h
    trunk/libstdc++-v3/include/parallel/base.h
    trunk/libstdc++-v3/include/parallel/equally_split.h
    trunk/libstdc++-v3/include/parallel/features.h
    trunk/libstdc++-v3/include/parallel/find.h
    trunk/libstdc++-v3/include/parallel/for_each.h
    trunk/libstdc++-v3/include/parallel/multiway_merge.h
    trunk/libstdc++-v3/include/parallel/multiway_mergesort.h
    trunk/libstdc++-v3/include/parallel/numeric
    trunk/libstdc++-v3/include/parallel/numericfwd.h
    trunk/libstdc++-v3/include/parallel/omp_loop.h
    trunk/libstdc++-v3/include/parallel/omp_loop_static.h
    trunk/libstdc++-v3/include/parallel/partial_sum.h
    trunk/libstdc++-v3/include/parallel/partition.h
    trunk/libstdc++-v3/include/parallel/quicksort.h
    trunk/libstdc++-v3/include/parallel/random_shuffle.h
    trunk/libstdc++-v3/include/parallel/settings.h
    trunk/libstdc++-v3/include/parallel/sort.h
    trunk/libstdc++-v3/include/parallel/tags.h
    trunk/libstdc++-v3/include/parallel/types.h
    trunk/libstdc++-v3/include/parallel/workstealing.h
    trunk/libstdc++-v3/src/Makefile.am
    trunk/libstdc++-v3/src/Makefile.in


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34797

[Bug libstdc++/34797] [parallel mode] Settings are separated for each compilation unit

[pcarlini at suse dot de]

------- Comment #7 from pcarlini at suse dot de  2008-02-21 17:06 -------
I understand this is fixed now. Otherwise, please reopen (and sorry ;)


-- 

pcarlini at suse dot de changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.3.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34797