Bug 42242 - vector::resize could be split up into two different functions
Summary: vector::resize could be split up into two different functions
Status: RESOLVED DUPLICATE of bug 32618
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 4.4.1
: P3 enhancement
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords: missed-optimization
Depends on:
Blocks:
 
Reported: 2009-12-01 18:14 UTC by Ivan Godard
Modified: 2009-12-02 00:01 UTC (History)
5 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ivan Godard 2009-12-01 18:14:31 UTC
This code shows that the resize member function is calling the null constructor even when the size does not need to be changed.

#include    <vector>
#include    <iostream>
class c {
public:
            c() { std::cerr << "c null constructor called\n"; }
            c(int i) : i(i) {}
    int     i;
    };

int main() {
    std::vector<c>f;
    f.push_back(c(0));
    f.resize(1);
    return 0;
    }
Comment 1 Jonathan Wakely 2009-12-01 18:21:53 UTC
this is caused by the default argument to resize:

void
resize(size_type __new_size, value_type __x = value_type());


It would be possible to avoid the default construction by replacing that member with two overloads of resize, but I'm not sure it's worth doing.
Comment 2 Jonathan Wakely 2009-12-01 18:30:51 UTC
On second thoughts, it might be necessary to split it into two overloads for C++1x, because this should work:

#include <vector>

struct moveable {
    explicit moveable(int) { }
    moveable(const moveable&) = delete;
    moveable(moveable&&) { }
};

int main()
{
    std::vector<moveable> v;
    moveable m(0);
    v.resize(1, m);
}

Comment 3 Andrew Pinski 2009-12-01 18:33:07 UTC
(In reply to comment #1) 
> It would be possible to avoid the default construction by replacing that member
> with two overloads of resize, but I'm not sure it's worth doing.

The C++03 standard only lists the function with the default value so this is invalid.
Comment 4 Andrew Pinski 2009-12-01 18:34:30 UTC
(In reply to comment #2)
> On second thoughts, it might be necessary to split it into two overloads for
> C++1x, because this should work:

But std::vector::resize (size_type sz, T c = T()); is the only one listed at least in C++03.  What does C++1x say about that function?  You might need to raise this to the C++ standards committee.
Comment 5 Jonathan Wakely 2009-12-01 18:37:13 UTC
(In reply to comment #2)
> On second thoughts, it might be necessary to split it into two overloads for
> C++1x, because this should work:

Gah, ignore that, I'm talking rubbish and that shouldn't work

Andrew, the standard would allow two overloads instead of one, see [member.functions]/2 - and the C++1x draft specifies two separate overloads.  It's a reasonable enhancement request IMHO, but necessarily not a bug.

Comment 6 Andrew Pinski 2009-12-01 18:40:57 UTC
(In reply to comment #5)
> (In reply to comment #2)
> > On second thoughts, it might be necessary to split it into two overloads for
> > C++1x, because this should work:
> 
> Gah, ignore that, I'm talking rubbish and that shouldn't work
> 
> Andrew, the standard would allow two overloads instead of one, see
> [member.functions]/2

hmm, I missed that :).

Note doesn't a change like this change the ABI?  Which means it has to wait until v7?
Comment 7 Jonathan Wakely 2009-12-01 18:54:47 UTC
Yes, it would be an ABI change, although that symbol isn't exported from the library.

vector::resize() was changed from one function to two in the n2284 draft, as a result of n1858, the rationale is the same as for deque, see
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2005/n1858.html#23.2.1.2%20-%20deque%20capacity
Comment 8 Chris Jefferson 2009-12-01 19:01:09 UTC
The only change in C++0x is that a new function is added, so it won't break any existing code, and we will be able to add it without moving to v7.
Comment 9 Ivan Godard 2009-12-01 19:02:07 UTC
FWIW, I think it is a design error in the standard. The resize function seems to be designed from an assumption that the vector type T is POD. The second argument should not be a T, it should be a constructor, so any added cells (if the resize extends the vector) are constructed in place rather than the construct-a-temp/copy-construct/destruct sequence. Or there should be no second argument and the null constructor used implicitly iff actually needed.

The actual usecase (that prompted the report) used the vector as a state holder in a recursive tree walk; the resize was used to cut back the state when returning from a leaf. The null constructor is seriously non-trivial for the node type, involving considerable file activity that could fail (a file system search). The workaround was simple when the problem was discovered: have a do-nothing null constructor for the vector, with the "real" null constructor turned into a constructor taking a dummy argument. Of course, that wound up getting exposed into the exported interface, and required macros to hide. :-(
Comment 10 Chris Jefferson 2009-12-01 19:07:26 UTC
Anyway, the result of this bug is that the fix to resize() given in the latest draft of C++0x should be applied. The resulting code would only be invoked if you ask the compiler to use C++0x mode, but if care about performance you should be doing this anyway, I suspect your types may well benefit from rvalue references.
Comment 11 Ivan Godard 2009-12-01 19:26:12 UTC
Subject: Re:  vector::resize could be split up into two
 different functions

We await r-value references with baited breath :-)

chris at bubblescope dot net wrote:
> ------- Comment #10 from chris at bubblescope dot net  2009-12-01 19:07 -------
> Anyway, the result of this bug is that the fix to resize() given in the latest
> draft of C++0x should be applied. The resulting code would only be invoked if
> you ask the compiler to use C++0x mode, but if care about performance you
> should be doing this anyway, I suspect your types may well benefit from rvalue
> references.
>
>
>   
Comment 12 Chris Jefferson 2009-12-01 19:28:53 UTC
No need to wait, they are in g++ 4.3 and 4.4, use -std=c++0x to activate features from C++0x. Obviously they are all subject to change, but are now fairly close to standardised.
Comment 13 Paolo Carlini 2009-12-02 00:01:07 UTC
This is essentially PR32618, see the last audit trail entries, in particular. As far as I know my last remark is still an issue (I told privately Howard but then I dropped it): without Concepts, it's quite hard to implement the two separate resize and at the same time keep on guaranteeing that the std::vector class can be explicitly instantiated for non-default constructible types. If somebody is aware of decently simple ways to do that for 4.5.0, I'm all ears...

*** This bug has been marked as a duplicate of 32618 ***