Re: [PATCH][libstdc++-v3 parallel mode] Make settings getter return non-const object

[Johannes Singler <singler at ira dot uka dot de>]

Hi,

> > This patch makes the getter method for the parallel mode settings 
> > structure return a non-const object, so it can be directly modified, 
> > which is much more convenient for the library user.
>
> I see where you are coming from, if one is trying to use the get()
> function to construct a modified _Settings object.
>
> Like so:
>
> using namespace __gnu_parallel;
> _Settings s = const_cast<_Settings>(_Settings::get());
> s.something = somenewthing;
> _Settings::set(s);
>
> This is what you are trying to avoid, hmmm?

Exactly. The user might want to change an option without modifying the 
other settings, possibly changed in another part of the program.
Many of the settings are quite "local" to specific functions, you rarely 
want to change a major part of the values at the same time.

We don't need the const_cast in this snippet, though, do we?
On the other hand, you could use const_cast to "break in".

> My thought was that this would be used more like:
>
> _Settings s;
> s.something = somenewthing;
> _Settings::set(s);
>
> See the updated documentation that was just checked in as part of
> libstdc++/35256.
>
> So, the question is, does this make sense? 

See above. Anyway, it's usually three lines instead of one.

> I do believe that there are solid codegen reasons for having the get()
> member return a const object, implying all the object members are const.
> By doing it this way, all the tuning information is constant at the
> site of use... this should allow the optimizers to elide the jump from
> get() and propagate the const configuration data. So, just like a
> macro! (?)

But the compiler should be able to tell that the value is *not* changed 
at that specific place, and to do the same optimizations. We could even 
have two get() variants. A const one and a non-const one.

> If the long-term goal is to force some of the parallelization
> heuristics into the compiler proper, (what I'm assuming), then doesn't
> this stuff have to be const at some point?

You mean that the user cannot change it at all?

> That may depend on IPA for C++ that is still not quite there yet. So,
> perhaps I'm getting ahead of myself...
>
> > AFAIK this does not change library binary compatibility, does it?
>
> Nope. It only changes the const qualification of the return
> type, which is not part of the mangled name. So, it's definitely safe
> to do this.

Good. So the question is still open. I see that there some argument for 
having it const, some against it. Before changing it back, we should 
really have discussed this point well, so I try to subsume the arguments 
at the moment:

_Settings::get() returns a const reference instead of a (modifiable) 
reference to the singleton object.

Pros:
-Changing the values can be forbidden to the user.
-Compiler can possibly better optimize read access.
-Possibility of having (p)thread-local settings (not the cleanest approach).

Cons:
-More complicated for the user to modify settings.
-A lot of copying a (considerably large and growing) structure around 
when modifying settings.

Possible compromises:
-Having a const getter and a non-const getter.

More comments (by others)?

-- Johannes