| Message ID | 1446554133-3090-1-git-send-email-aurelio.remonda@tallertechnologies.com |
|---|---|
| State | New |
| Headers | show |
Hi, On 11/03/2015 01:35 PM, Aurelio Remonda wrote: > diff --git a/ChangeLog b/ChangeLog > index 5b16ca2..a1cd0d3 100644 > --- a/ChangeLog > +++ b/ChangeLog > @@ -1,3 +1,12 @@ > +2015-10-30 Aurelio Remonda <aurelio.remonda@tallertechnologies.com> > + > + * libstdc++-v3/acinclude.m4: add enable_new_opnt_no_allocation_retry > + flag definition. > + * libstdc++-v3/configure.ac: add option flag > + GLIBCXX_ENABLE_NEW_OPNT_NO_ALLOCATION_RETRY > + * libstdc++-v3/libsupc++/new_opnt.cc use the defined macro > + * libstdc++-v3/doc/xml/manual/configure.xml > + Three minor comments. First, ChangeLog entries aren't normally submitted as part of the patch. Second, since the ChangeLog is under libstdc++-v3, the ChangeLog entries should not have libstdc++-v3 in the paths (eg, just * acinclude.m4: ...). Finally, since you are touching acinclude.m4 you should normally run autoreconf, mention in the ChangeLog the changed regenerated files and eventually commit those changes too (like the ChangeLog entries, those aren't normally part of the posted patch) About the three issues, you have plenty of examples in the mailing list. Otherwise, about the substance of the patch, I think we want to wait for Jonathan to be back. Paolo.
On Tue, Nov 3, 2015 at 10:26 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote: > Hi, > > On 11/03/2015 01:35 PM, Aurelio Remonda wrote: >> >> diff --git a/ChangeLog b/ChangeLog >> index 5b16ca2..a1cd0d3 100644 >> --- a/ChangeLog >> +++ b/ChangeLog >> @@ -1,3 +1,12 @@ >> +2015-10-30 Aurelio Remonda <aurelio.remonda@tallertechnologies.com> >> + >> + * libstdc++-v3/acinclude.m4: add >> enable_new_opnt_no_allocation_retry >> + flag definition. >> + * libstdc++-v3/configure.ac: add option flag >> + GLIBCXX_ENABLE_NEW_OPNT_NO_ALLOCATION_RETRY >> + * libstdc++-v3/libsupc++/new_opnt.cc use the defined macro >> + * libstdc++-v3/doc/xml/manual/configure.xml >> + > > Three minor comments. First, ChangeLog entries aren't normally submitted as > part of the patch. Second, since the ChangeLog is under libstdc++-v3, the > ChangeLog entries should not have libstdc++-v3 in the paths (eg, just * > acinclude.m4: ...). Ok, so ChangeLog modifications should be another patch? >Finally, since you are touching acinclude.m4 you should > normally run autoreconf, mention in the ChangeLog the changed regenerated > files and eventually commit those changes too (like the ChangeLog entries, > those aren't normally part of the posted patch) About the three issues, you > have plenty of examples in the mailing list. I have a problem with autoreconf, when i run it with autoconf 2.69 it says i need exactly autoconf 2.64 so i install it and try to do autoreconf with 2.64 and this is what i get: aurelio-remonda@Remonda-PC:~/gcc/libstdc++-v3$ autoreconf configure.ac:74: error: Autoconf version 2.65 or higher is required You can see i am using my recently installed autoconf: aurelio-remonda@Remonda-PC:~/gcc/libstdc++-v3$ which autoconf /home/aurelio-remonda/autoconf-2.64/install/bin/autoconf I even try 2.65: aurelio-remonda@Remonda-PC:~/gcc/libstdc++-v3$ which autoreconf /home/aurelio-remonda/autoconf-2.65/install/bin/autoreconf and got this: aurelio-remonda@Remonda-PC:~/gcc/libstdc++-v3$ autoreconf configure.ac:4: error: Please use exactly Autoconf 2.64 instead of 2.65. ../config/override.m4:12: _GCC_AUTOCONF_VERSION_CHECK is expanded from... configure.ac:4: the top level autom4te: /usr/bin/m4 failed with exit status: 1 aclocal: error: echo failed with exit status: 1 autoreconf: aclocal failed with exit status: 1 So i changed the version here: /src/gcc/libstdc++-v3/configure.ac:3 AC_PREREQ(2.69) And here: /src/config/override.m4:59 m4_if(m4_defn([m4_PACKAGE_VERSION]), [2.69] And did the autoreconf with 2.69 and that worked. But Jonathan told me that i don't have to make those changes.
Aurelio Remonda <aurelio.remonda@tallertechnologies.com> writes: > aurelio-remonda@Remonda-PC:~/gcc/libstdc++-v3$ autoreconf > configure.ac:74: error: Autoconf version 2.65 or higher is required Make sure you have automake 1.11.6. Andreas.
On Tue, Nov 3, 2015 at 10:26 AM, Paolo Carlini <paolo.carlini@oracle.com> wrote: > Finally, since you are touching acinclude.m4 you should > normally run autoreconf, mention in the ChangeLog the changed regenerated > files and eventually commit those changes too (like the ChangeLog entries, > those aren't normally part of the posted patch) About the three issues, you > have plenty of examples in the mailing list. > > Otherwise, about the substance of the patch, I think we want to wait for > Jonathan to be back. > > Paolo. Just to be sure we are in the same page, i dont have commit access. On the other hand I'm not quite sure I follow: -The changes on the regenerated files have to be part of the patch? (i.e the configure file changes after doing autoreconf) Thank you!
On 11/03/2015 05:35 AM, Aurelio Remonda wrote: > Currently, whenever operator new (std::nothrow) fails to allocate memory, it'll > check if there is a new-handler function available. If there is, it'll call > the handler and then try to allocate again. Otherwise, it'll return a null pointer. > > This retrying behavior may not always be desirable. If the handler cannot fix > the memory allocation issue, we may end up being stuck in an infinite loop. > Whereas returning nullptr may be a valid alternative to keep calling the new_handler. > The workaround to end the loop, we would have to call std::set_new_handler(nullptr) > from within the handler itself, which gets complicated if the handler has to be > re-setted afterwards. > > This patch adds the new_nothrow_no_retry configuration flag, which, if enabled, > will change the retrying behavior of operator new (std::nothrow) so that it only calls > the handler once when it fails to allocate memory and the return nullptr. The purpose of the loop is to give the new handler an opportunity to free up enough memory to let the allocation succeed. Since the handler doesn't get passed the size of the request it has no easy way of determining how much memory to free. The loop lets it free up increasingly more memory. If it can't free up any memory it is expected/required to either indicate failure by throwing bad_alloc or terminate the process. It's not allowed to return otherwise. Besides violating the requirement of the C++ standard, replacing the loop with an if statement would disable that aspect of the feature for the whole system (or for all processes that link with the libstdc++ that was configured this way). It would effectively be imposing a system-wide policy without providing a mechanism for correctly written programs to opt out. In addition, as a configuration option, it could not be easily tested. I would therefore advise against making such a change. Martin
On Tue, Nov 3, 2015 at 5:25 PM, Martin Sebor <msebor@gmail.com> wrote: > On 11/03/2015 05:35 AM, Aurelio Remonda wrote: >> >> Currently, whenever operator new (std::nothrow) fails to allocate memory, >> it'll >> check if there is a new-handler function available. If there is, it'll >> call >> the handler and then try to allocate again. Otherwise, it'll return a null >> pointer. >> >> This retrying behavior may not always be desirable. If the handler cannot >> fix >> the memory allocation issue, we may end up being stuck in an infinite >> loop. >> Whereas returning nullptr may be a valid alternative to keep calling the >> new_handler. >> The workaround to end the loop, we would have to call >> std::set_new_handler(nullptr) >> from within the handler itself, which gets complicated if the handler has >> to be >> re-setted afterwards. >> >> This patch adds the new_nothrow_no_retry configuration flag, which, if >> enabled, >> will change the retrying behavior of operator new (std::nothrow) so that >> it only calls >> the handler once when it fails to allocate memory and the return nullptr. > > > The purpose of the loop is to give the new handler an opportunity > to free up enough memory to let the allocation succeed. Since the > handler doesn't get passed the size of the request it has no easy > way of determining how much memory to free. The loop lets it free > up increasingly more memory. If it can't free up any memory it is > expected/required to either indicate failure by throwing bad_alloc Since this is a nothrow new, we thought that probably the system might not be exceptions-friendly (such as certain embedded systems), so we wanted to provide the new_handler the ability to do something else other than trying to allocate memory and keep the function iterating. In fact, our idea is that since the nothrow new can indeed return nullptr, let the new_handler do something else and leave the no-memory-consequence to the caller. This new flag enables just that. > or terminate the process. It's not allowed to return otherwise. > > Besides violating the requirement of the C++ standard, replacing Could you please point us to the relevant section where this behavior is enforced? We couldn't find it so far. > the loop with an if statement would disable that aspect of the > feature for the whole system (or for all processes that link with > the libstdc++ that was configured this way). It would effectively > be imposing a system-wide policy without providing a mechanism > for correctly written programs to opt out. In addition, as > a configuration option, it could not be easily tested. I would > therefore advise against making such a change. > > Martin
>> Besides violating the requirement of the C++ standard, replacing > > Could you please point us to the relevant section where this behavior > is enforced? We couldn't find it so far. The Required behavior of the nothrow operator new reads: This nothrow version of operator new returns a pointer obtained as if acquired from the (possibly replaced) ordinary version... The "as if" requirement implies that any observable effects of "the (possibly replaced) ordinary version" must be preserved. The repeated calls to the new handler are among such effects. Martin
On Nov 3, 2015, at 1:10 PM, Martin Sebor <msebor@gmail.com> wrote: > The "as if" requirement implies that any observable effects of > "the (possibly replaced) ordinary version" must be preserved. > The repeated calls to the new handler are among such effects. Unless the standard is fixed to say that one cannot observe the repeated calls. We do this in some places, for some things: 15Whenever a temporary class object is copied using a copy constructor, and this object and the copy have the same cv-unqualified type, an implementation is permitted to treat the original and the copy as two different ways of referring to the same object and not perform a copy at all, even if the class copy constructor or destructor have side effects. For a function with a class return type, if the expression in the return statement is the name of a local object, and the cv- unqualified type of the local object is the same as the function return type, an implementation is permitted to omit creating the tem- porary object to hold the function return value, even if the class copy constructor or destructor has side effects. In these cases, the object is destroyed at the later of times when the original and the copy would have been destroyed without the optimization.111) in C++, so, it isn’t out of the question. I was looking for dynamic -> static object optimization wording, but didn’t find it in the first C++ standard. That is a fairly reasonable thing to do, and if done well, can reasonably change the observable side-effects as well.
On 11/03/2015 04:08 PM, Mike Stump wrote: > On Nov 3, 2015, at 1:10 PM, Martin Sebor <msebor@gmail.com> wrote: >> The "as if" requirement implies that any observable effects of >> "the (possibly replaced) ordinary version" must be preserved. >> The repeated calls to the new handler are among such effects. > > Unless the standard is fixed to say that one cannot observe the repeated calls. We do this in some places, for some things: There are two sets of important observable effects: the calls to the handler, and the call to the (possibly replaced) ordinary operator new. A C++ program is allowed to replace just the ordinary operator new (and not the nothrow version), call the default nothrow operator new, and expect to get back the same pointer that would have been returned from the replaced new had it been called instead. (Some implementations, including libstdc++, fail to conform to this requirement.) Changing the standard to remove the requirement to preserve these effects would break the portability of programs that rely on it. Martin
On 4 November 2015 at 01:55, Martin Sebor wrote: > On 11/03/2015 05:35 AM, Aurelio Remonda wrote: >> >> Currently, whenever operator new (std::nothrow) fails to allocate memory, >> it'll >> check if there is a new-handler function available. If there is, it'll >> call >> the handler and then try to allocate again. Otherwise, it'll return a null >> pointer. >> >> This retrying behavior may not always be desirable. If the handler cannot >> fix >> the memory allocation issue, we may end up being stuck in an infinite >> loop. >> Whereas returning nullptr may be a valid alternative to keep calling the >> new_handler. >> The workaround to end the loop, we would have to call >> std::set_new_handler(nullptr) >> from within the handler itself, which gets complicated if the handler has >> to be >> re-setted afterwards. >> >> This patch adds the new_nothrow_no_retry configuration flag, which, if >> enabled, >> will change the retrying behavior of operator new (std::nothrow) so that >> it only calls >> the handler once when it fails to allocate memory and the return nullptr. > > > The purpose of the loop is to give the new handler an opportunity > to free up enough memory to let the allocation succeed. Since the > handler doesn't get passed the size of the request it has no easy > way of determining how much memory to free. The loop lets it free > up increasingly more memory. If it can't free up any memory it is > expected/required to either indicate failure by throwing bad_alloc > or terminate the process. It's not allowed to return otherwise. > > Besides violating the requirement of the C++ standard, replacing > the loop with an if statement would disable that aspect of the > feature for the whole system (or for all processes that link with > the libstdc++ that was configured this way). It would effectively > be imposing a system-wide policy without providing a mechanism > for correctly written programs to opt out. In addition, as > a configuration option, it could not be easily tested. I would > therefore advise against making such a change. I share your concerns, but I'm also sympathetic to the changes that the Taller Technologies team are trying to make, to allow libstdc++ to be more useful in exception-free systems. At the very least the patch to doc/xml/manual/configure.xml must document that this option enables behaviour that violates the standard and so produces a non-conforming implementation. It should probably also explain how to use the resulting implementation, so that users on embedded systems who might want to enable this know how to write a suitable new-handler.
On 4 November 2015 at 02:11, Daniel Gutson wrote: > Since this is a nothrow new, we thought that probably the system > might not be exceptions-friendly (such as certain embedded systems), > so we wanted to provide the new_handler the ability to do something else > other than trying to allocate memory and keep the function iterating. That could be done using an alternative overload of operator new instead of altering the semantics of the standard one (that could be provided as a GNU extension, for example). > In fact, our idea is that since the nothrow new can indeed return nullptr, > let the new_handler do something else and leave the no-memory-consequence > to the caller. > This new flag enables just that. The default configuration already allows the caller to deal with allocation failure from the nothrow version of new, by not installing a new-handler installed, and dealing with a null return value. How would I use this alternative configuration? Since the behaviour only changes when a new-handler is installed, presumably I'm meant to install some kind of new-handler that does something else ... but what? The patch should include documentation explaining when and how to use this option.
On 4 November 2015 at 11:50, Jonathan Wakely wrote: > On 4 November 2015 at 02:11, Daniel Gutson wrote: >> Since this is a nothrow new, we thought that probably the system >> might not be exceptions-friendly (such as certain embedded systems), >> so we wanted to provide the new_handler the ability to do something else >> other than trying to allocate memory and keep the function iterating. > > That could be done using an alternative overload of operator new > instead of altering the semantics of the standard one (that could be > provided as a GNU extension, for example). Another option would be to provide a new-handler that enables the desired behaviour e.g. void __no_retry_new_handler() { __throw_bad_alloc(); } ... while (__builtin_expect ((p = malloc (sz)) == 0, false)) { new_handler handler = std::get_new_handler (); if (! handler || handler == __no_retry_new_handler) return 0; The caller would then be responsible for attempting to make more memory available (rather than the new-handler doing it).
On Tue, 3 Nov 2015, Mike Stump wrote: > On Nov 3, 2015, at 1:10 PM, Martin Sebor <msebor@gmail.com> wrote: >> The "as if" requirement implies that any observable effects of >> "the (possibly replaced) ordinary version" must be preserved. >> The repeated calls to the new handler are among such effects. > > Unless the standard is fixed to say that one cannot observe the repeated > calls. We do this in some places, for some things: [snip copy elision] > in C++, so, it isn’t out of the question. I was looking for dynamic -> > static object optimization wording, but didn’t find it in the first C++ > standard. That is a fairly reasonable thing to do, and if done well, > can reasonably change the observable side-effects as well. There is recent-ish wording in [expr.new] that says that 'new int' is not forced to call operator new. However, operator new (which can be called directly) has a rather strictly defined behavior. And a bad one at that, since it forces the use of an exception to report an error to the direct caller... On Tue, 3 Nov 2015, Martin Sebor wrote: > There are two sets of important observable effects: the calls to > the handler, and the call to the (possibly replaced) ordinary > operator new. "This nothrow version of operator new returns a pointer obtained as if acquired from the (possibly replaced) ordinary version." For a non-native speaker, it is extremely unclear if the (possibly replaced) ordinary version is the original one that has been displaced or if it is the user one that did the replacement. > A C++ program is allowed to replace just the ordinary operator > new (and not the nothrow version), call the default nothrow > operator new, and expect to get back the same pointer that would > have been returned from the replaced new had it been called instead. > (Some implementations, including libstdc++, fail to conform to this > requirement.) That's also the interpretation I remember, and I find it contradictory with the C++ philosophy that exceptions are heavy, expensive, and should be avoided for local use. (of course allocation failure should be rare)
> I share your concerns, but I'm also sympathetic to the changes that > the Taller Technologies team are trying to make, to allow libstdc++ to > be more useful in exception-free systems. > > At the very least the patch to doc/xml/manual/configure.xml must > document that this option enables behaviour that violates the standard > and so produces a non-conforming implementation. It should probably > also explain how to use the resulting implementation, so that users on > embedded systems who might want to enable this know how to write a > suitable new-handler. If there is a problem with programs too easily getting themselves into an infinite loop because of improperly written new handlers it might be worth proposing a change to C++ to make the minimum number of iterations the loop must make before giving up implementation-defined. That way this could be handled the same way regardless of the environment. Martin
On Wed, Nov 4, 2015 at 3:20 AM, Jonathan Wakely <jwakely.gcc@gmail.com> wrote: > On 4 November 2015 at 02:11, Daniel Gutson wrote: >> Since this is a nothrow new, we thought that probably the system >> might not be exceptions-friendly (such as certain embedded systems), >> so we wanted to provide the new_handler the ability to do something else >> other than trying to allocate memory and keep the function iterating. > > That could be done using an alternative overload of operator new > instead of altering the semantics of the standard one (that could be > provided as a GNU extension, for example). > >> In fact, our idea is that since the nothrow new can indeed return nullptr, >> let the new_handler do something else and leave the no-memory-consequence >> to the caller. >> This new flag enables just that. > > > The default configuration already allows the caller to deal with > allocation failure from the nothrow version of new, by not installing > a new-handler installed, and dealing with a null return value. How > would I use this alternative configuration? Since the behaviour only > changes when a new-handler is installed, presumably I'm meant to > install some kind of new-handler that does something else ... but > what? The patch should include documentation explaining when and how > to use this option. Real use cases: statistics and logging. It's a (one time) callback reporting that something went wrong, but not intended to fix things e.g. by attempting to free more memory.
On 5 November 2015 at 20:52, Daniel Gutson wrote: > Real use cases: statistics and logging. It's a (one time) callback > reporting that something went wrong, > but not intended to fix things e.g. by attempting to free more memory. Why can't that be done by replacing operator new with a user-defined function, instead of modifying the default one? A program that wants to make use of the altered semantics is non-portable because it relies on a custom libstdc++ configuration option, and has to install a custom new-handler that relies on the altered libstdc++ behaviour. Replacing operator new to do the logging is portable and requires no custom configuration. (I'm not rejecting the patch, but am still trying to make sure it's a useful change.)
On Thu, Nov 5, 2015 at 2:11 PM, Jonathan Wakely <jwakely.gcc@gmail.com> wrote: > On 5 November 2015 at 20:52, Daniel Gutson wrote: >> Real use cases: statistics and logging. It's a (one time) callback >> reporting that something went wrong, >> but not intended to fix things e.g. by attempting to free more memory. > > Why can't that be done by replacing operator new with a user-defined > function, instead of modifying the default one? > > A program that wants to make use of the altered semantics is > non-portable because it relies on a custom libstdc++ configuration > option, and has to install a custom new-handler that relies on the > altered libstdc++ behaviour. Replacing operator new to do the logging > is portable and requires no custom configuration. > > (I'm not rejecting the patch, but am still trying to make sure it's a > useful change.) The issue is, as I understand it, to do the actual work of operator new, i.e. allocate memory. It should force us to copy most of the code of the original code of operator new, which may change on new versions of the STL, forcing us to keep updated. This is important when having legacy code that uses operator new (but doesn't use a new_handler),
On 5 November 2015 at 23:31, Daniel Gutson <daniel.gutson@tallertechnologies.com> wrote: > On Thu, Nov 5, 2015 at 2:11 PM, Jonathan Wakely <jwakely.gcc@gmail.com> wrote: >> On 5 November 2015 at 20:52, Daniel Gutson wrote: >>> Real use cases: statistics and logging. It's a (one time) callback >>> reporting that something went wrong, >>> but not intended to fix things e.g. by attempting to free more memory. >> >> Why can't that be done by replacing operator new with a user-defined >> function, instead of modifying the default one? >> >> A program that wants to make use of the altered semantics is >> non-portable because it relies on a custom libstdc++ configuration >> option, and has to install a custom new-handler that relies on the >> altered libstdc++ behaviour. Replacing operator new to do the logging >> is portable and requires no custom configuration. >> >> (I'm not rejecting the patch, but am still trying to make sure it's a >> useful change.) > > The issue is, as I understand it, to do the actual work of operator > new, i.e. allocate memory. It should force > us to copy most of the code of the original code of operator new, > which may change on new versions of the > STL, forcing us to keep updated. It can just call malloc, and the replacement operator delete can call free. That is very unlikely to need to change (which is corroborated by the fact that the default definitions in libsupc++ change very rarely). Relying on a non-standard configuration of a single implementation seems far more fragile than writing a custom new/delete. > This is important when having legacy code that uses operator new (but > doesn't use a new_handler), Such code won't be affected by the proposed patch anyway, because if there is no new-handler then there is no change in behaviour, it returns after the first call to malloc.
On 6 November 2015 at 09:02, Daniel Gutson <daniel.gutson@tallertechnologies.com> wrote: > > El 5/11/2015 22:56, "Jonathan Wakely" <jwakely.gcc@gmail.com> escribió: >> >> On 5 November 2015 at 23:31, Daniel Gutson >> <daniel.gutson@tallertechnologies.com> wrote: >> > On Thu, Nov 5, 2015 at 2:11 PM, Jonathan Wakely <jwakely.gcc@gmail.com> >> > wrote: >> >> On 5 November 2015 at 20:52, Daniel Gutson wrote: >> >>> Real use cases: statistics and logging. It's a (one time) callback >> >>> reporting that something went wrong, >> >>> but not intended to fix things e.g. by attempting to free more memory. >> >> >> >> Why can't that be done by replacing operator new with a user-defined >> >> function, instead of modifying the default one? >> >> >> >> A program that wants to make use of the altered semantics is >> >> non-portable because it relies on a custom libstdc++ configuration >> >> option, and has to install a custom new-handler that relies on the >> >> altered libstdc++ behaviour. Replacing operator new to do the logging >> >> is portable and requires no custom configuration. >> >> >> >> (I'm not rejecting the patch, but am still trying to make sure it's a >> >> useful change.) >> > >> > The issue is, as I understand it, to do the actual work of operator >> > new, i.e. allocate memory. It should force >> > us to copy most of the code of the original code of operator new, >> > which may change on new versions of the >> > STL, forcing us to keep updated. >> >> It can just call malloc, and the replacement operator delete can call >> free. > > It can but the actual code of operator new is optimized and more complex > (for example by using intrinsics). The user might not want to deal with the > STL implementation internals. By this argument users should never replace operator new at all. I don't find that convincing. If they don't want to deal with implementation details then they don't have to. The default implementations are optimized because they are used by everyone and it would be silly to ship them without making easy optimisations, but that doesn't mean that users have to do the same thing in their own replacement allocation functions. Using __builtin_expect makes a small difference when looping on the result of malloc, but if you were to provide a replacement that doesn't loop then the difference for a single branch is not very significant. If profiling shows it matters then it's easy to use __builtin_expect (it's not an implementation detail, it's a public function documented in the manual and available for users, and most code bases I've worked on have used it somewhere). >> >> That is very unlikely to need to change (which is corroborated by the >> fact that the default definitions in libsupc++ change very rarely). >> >> Relying on a non-standard configuration of a single implementation >> seems far more fragile than writing a custom new/delete. >> >> > This is important when having legacy code that uses operator new (but >> > doesn't use a new_handler), >> >> Such code won't be affected by the proposed patch anyway, because if >> there is no new-handler then there is no change in behaviour, it >> returns after the first call to malloc. > > I didn't explain myself correctly: what I meant is that this is useful when > dealing with legacy library that does not use a new_handler, which gives > room to define the one for the statistics, logging and leds diagnostics in > the user code. Indeed the library code doesn't get affected, but get > "sniffed" by the user's handler. OK, but a user's replacement operator new would have the same effect, without affecting the library code.
On Fri, 6 Nov 2015, Jonathan Wakely wrote: > On 6 November 2015 at 09:02, Daniel Gutson > <daniel.gutson@tallertechnologies.com> wrote: >> >> El 5/11/2015 22:56, "Jonathan Wakely" <jwakely.gcc@gmail.com> escribió: >>> >>> It can just call malloc, and the replacement operator delete can call >>> free. >> >> It can but the actual code of operator new is optimized and more complex >> (for example by using intrinsics). The user might not want to deal with the >> STL implementation internals. The code is more complex for reasons that might not apply to your code. If you are replacing new, you don't need to look at a new handler, you can write directly whatever code you want. If you know how malloc(0) behaves on your platform (or are confident that you will never call new with size 0), you can skip the initial test. IIRC I introduced the __builtin_expect in that code, but I personnally often write the following in my code, assuming that allocation will never fail: inline void* operator new(std::size_t n){return malloc(n);} inline void operator delete(void*p)noexcept{free(p);} (same with nothrow and arrays) (technically illegal because of 'inline' but I don't care) > By this argument users should never replace operator new at all. I > don't find that convincing. > > If they don't want to deal with implementation details then they don't > have to. The default implementations are optimized because they are > used by everyone and it would be silly to ship them without making > easy optimisations, but that doesn't mean that users have to do the > same thing in their own replacement allocation functions. Using > __builtin_expect makes a small difference when looping on the result > of malloc, but if you were to provide a replacement that doesn't loop > then the difference for a single branch is not very significant. Actually, it is optimized for the case where it does *not* loop (gcc by default optimizes by assuming that loops do loop).
On 11/06/2015 01:56 AM, Jonathan Wakely wrote: > On 5 November 2015 at 23:31, Daniel Gutson >> The issue is, as I understand it, to do the actual work of operator >> new, i.e. allocate memory. It should force >> us to copy most of the code of the original code of operator new, >> which may change on new versions of the >> STL, forcing us to keep updated. > > It can just call malloc, and the replacement operator delete can call free. > > That is very unlikely to need to change (which is corroborated by the > fact that the default definitions in libsupc++ change very rarely). Or perhaps libsupc++ could provide the default operator new under a __default_operator_new alias or some such, so that the user-defined replacement can fallback to calling it. Likewise for op delete. Thanks, Pedro Alves
On 06/11/15 09:59 +0000, Pedro Alves wrote: >On 11/06/2015 01:56 AM, Jonathan Wakely wrote: >> On 5 November 2015 at 23:31, Daniel Gutson > >>> The issue is, as I understand it, to do the actual work of operator >>> new, i.e. allocate memory. It should force >>> us to copy most of the code of the original code of operator new, >>> which may change on new versions of the >>> STL, forcing us to keep updated. >> >> It can just call malloc, and the replacement operator delete can call free. >> >> That is very unlikely to need to change (which is corroborated by the >> fact that the default definitions in libsupc++ change very rarely). > >Or perhaps libsupc++ could provide the default operator new under >a __default_operator_new alias or some such, so that the user-defined >replacement can fallback to calling it. Likewise for op delete. That could be useful, please file an enhancement request in bugzilla if you'd like that done.
On 11/10/2015 01:10 PM, Jonathan Wakely wrote: > On 06/11/15 09:59 +0000, Pedro Alves wrote: >> On 11/06/2015 01:56 AM, Jonathan Wakely wrote: >>> On 5 November 2015 at 23:31, Daniel Gutson >> >>>> The issue is, as I understand it, to do the actual work of operator >>>> new, i.e. allocate memory. It should force >>>> us to copy most of the code of the original code of operator new, >>>> which may change on new versions of the >>>> STL, forcing us to keep updated. >>> >>> It can just call malloc, and the replacement operator delete can call free. >>> >>> That is very unlikely to need to change (which is corroborated by the >>> fact that the default definitions in libsupc++ change very rarely). >> >> Or perhaps libsupc++ could provide the default operator new under >> a __default_operator_new alias or some such, so that the user-defined >> replacement can fallback to calling it. Likewise for op delete. > > That could be useful, please file an enhancement request in bugzilla > if you'd like that done. > I'll leave that to Daniel/Aurelio. Thanks, Pedro Alves
On 05/11/15 16:22, Daniel Gutson wrote: > On Wed, Nov 4, 2015 at 3:20 AM, Jonathan Wakely<jwakely.gcc@gmail.com> wrote: >> >On 4 November 2015 at 02:11, Daniel Gutson wrote: >>> >>Since this is a nothrow new, we thought that probably the system >>> >>might not be exceptions-friendly (such as certain embedded systems), >>> >>so we wanted to provide the new_handler the ability to do something else >>> >>other than trying to allocate memory and keep the function iterating. >> > >> >That could be done using an alternative overload of operator new >> >instead of altering the semantics of the standard one (that could be >> >provided as a GNU extension, for example). >> > >>> >>In fact, our idea is that since the nothrow new can indeed return nullptr, >>> >>let the new_handler do something else and leave the no-memory-consequence >>> >>to the caller. >>> >>This new flag enables just that. >> > >> > >> >The default configuration already allows the caller to deal with >> >allocation failure from the nothrow version of new, by not installing >> >a new-handler installed, and dealing with a null return value. How >> >would I use this alternative configuration? Since the behaviour only >> >changes when a new-handler is installed, presumably I'm meant to >> >install some kind of new-handler that does something else ... but >> >what? The patch should include documentation explaining when and how >> >to use this option. > Real use cases: statistics and logging. It's a (one time) callback > reporting that something went wrong, > but not intended to fix things e.g. by attempting to free more memory. For statistics and logging you may also use the --wrap option of GNU ld.
On Tue, Nov 10, 2015 at 10:10 AM, Jonathan Wakely <jwakely@redhat.com> wrote: > On 06/11/15 09:59 +0000, Pedro Alves wrote: >> >> On 11/06/2015 01:56 AM, Jonathan Wakely wrote: >>> >>> On 5 November 2015 at 23:31, Daniel Gutson >> >> >>>> The issue is, as I understand it, to do the actual work of operator >>>> new, i.e. allocate memory. It should force >>>> us to copy most of the code of the original code of operator new, >>>> which may change on new versions of the >>>> STL, forcing us to keep updated. >>> >>> >>> It can just call malloc, and the replacement operator delete can call >>> free. >>> >>> That is very unlikely to need to change (which is corroborated by the >>> fact that the default definitions in libsupc++ change very rarely). >> >> >> Or perhaps libsupc++ could provide the default operator new under >> a __default_operator_new alias or some such, so that the user-defined >> replacement can fallback to calling it. Likewise for op delete. that would allow us to overload operator new as something like this: void* operator new ( std::size_t count, const std::nothrow_t& tag) noexcept(true) { const auto old_handler = std::set_new_handler(nullptr); const auto ret = __default_operator_new(count, tag); std::set_new_handler(old_handler); return ret; } This is a non-iterating operator new. This additional user defined operator new would be possible: void* operator new ( std::size_t count, const std::nothrow_t& tag) noexcept(true) { const auto old_handler = std::set_new_handler(nullptr); const auto ret = __default_operator_new(count, tag); std::set_new_handler(old_handler); if (ret == nullptr && old_handler != nullptr) old_handler(); return ret; } So I like the idea. > > > That could be useful, please file an enhancement request in bugzilla > if you'd like that done.
On Mon, Nov 16, 2015 at 3:56 PM, Pedro Alves <palves@redhat.com> wrote: > On 11/10/2015 01:10 PM, Jonathan Wakely wrote: >> On 06/11/15 09:59 +0000, Pedro Alves wrote: >>> On 11/06/2015 01:56 AM, Jonathan Wakely wrote: >>>> On 5 November 2015 at 23:31, Daniel Gutson >>> >>>>> The issue is, as I understand it, to do the actual work of operator >>>>> new, i.e. allocate memory. It should force >>>>> us to copy most of the code of the original code of operator new, >>>>> which may change on new versions of the >>>>> STL, forcing us to keep updated. >>>> >>>> It can just call malloc, and the replacement operator delete can call free. >>>> >>>> That is very unlikely to need to change (which is corroborated by the >>>> fact that the default definitions in libsupc++ change very rarely). >>> >>> Or perhaps libsupc++ could provide the default operator new under >>> a __default_operator_new alias or some such, so that the user-defined >>> replacement can fallback to calling it. Likewise for op delete. >> >> That could be useful, please file an enhancement request in bugzilla >> if you'd like that done. >> > > I'll leave that to Daniel/Aurelio. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69879 Please assign it to Aurelio. Thanks and thanks Pedro for the idea. Daniel. > > Thanks, > Pedro Alves >
On 19/02/16 18:56 -0300, Daniel Gutson wrote: >On Mon, Nov 16, 2015 at 3:56 PM, Pedro Alves <palves@redhat.com> wrote: >> On 11/10/2015 01:10 PM, Jonathan Wakely wrote: >>> On 06/11/15 09:59 +0000, Pedro Alves wrote: >>>> On 11/06/2015 01:56 AM, Jonathan Wakely wrote: >>>>> On 5 November 2015 at 23:31, Daniel Gutson >>>> >>>>>> The issue is, as I understand it, to do the actual work of operator >>>>>> new, i.e. allocate memory. It should force >>>>>> us to copy most of the code of the original code of operator new, >>>>>> which may change on new versions of the >>>>>> STL, forcing us to keep updated. >>>>> >>>>> It can just call malloc, and the replacement operator delete can call free. >>>>> >>>>> That is very unlikely to need to change (which is corroborated by the >>>>> fact that the default definitions in libsupc++ change very rarely). >>>> >>>> Or perhaps libsupc++ could provide the default operator new under >>>> a __default_operator_new alias or some such, so that the user-defined >>>> replacement can fallback to calling it. Likewise for op delete. >>> >>> That could be useful, please file an enhancement request in bugzilla >>> if you'd like that done. >>> >> >> I'll leave that to Daniel/Aurelio. > >https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69879 > >Please assign it to Aurelio. Done, I will look forward to seeing some patches for the next stage 1. Thanks!
diff --git a/ChangeLog b/ChangeLog index 5b16ca2..a1cd0d3 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,12 @@ +2015-10-30 Aurelio Remonda <aurelio.remonda@tallertechnologies.com> + + * libstdc++-v3/acinclude.m4: add enable_new_opnt_no_allocation_retry + flag definition. + * libstdc++-v3/configure.ac: add option flag + GLIBCXX_ENABLE_NEW_OPNT_NO_ALLOCATION_RETRY + * libstdc++-v3/libsupc++/new_opnt.cc use the defined macro + * libstdc++-v3/doc/xml/manual/configure.xml + 2015-10-09 Martin Liska <mliska@suse.cz> * MAINTAINERS (Write After Approval): Add myself. diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4 index abf2e93..c8f7a75 100644 --- a/libstdc++-v3/acinclude.m4 +++ b/libstdc++-v3/acinclude.m4 @@ -2629,6 +2629,15 @@ AC_DEFUN([GLIBCXX_ENABLE_LONG_LONG], [ AC_MSG_RESULT([$enable_long_long]) ]) +AC_DEFUN([GLIBCXX_ENABLE_NEW_OPNT_NO_ALLOCATION_RETRY], [ + GLIBCXX_ENABLE(new-opnt-no-allocation-retry,$1,,[enable new nothrow no allocation retry condition]) + if test $enable_new_opnt_no_allocation_retry = yes; then + AC_DEFINE(_GLIBCXX_NEW_OPNT_NO_ALLOCATION_RETRY, 1, + [Define if operator new (std::nothrow) will retry allocation after callin the handler.]) + fi + AC_MSG_CHECKING([whether no-throw operator new should retry allocation after calling the new handler]) + AC_MSG_RESULT([$enable_new_opnt_no_allocation_retry]) +]) dnl dnl Check for decimal floating point. diff --git a/libstdc++-v3/configure.ac b/libstdc++-v3/configure.ac index 3456348..2f3aaa9 100644 --- a/libstdc++-v3/configure.ac +++ b/libstdc++-v3/configure.ac @@ -165,6 +165,7 @@ GLIBCXX_ENABLE_CLOCALE GLIBCXX_ENABLE_ALLOCATOR GLIBCXX_ENABLE_CHEADERS($c_model) dnl c_model from configure.host GLIBCXX_ENABLE_LONG_LONG([yes]) +GLIBCXX_ENABLE_NEW_OPNT_NO_ALLOCATION_RETRY([no]) GLIBCXX_ENABLE_WCHAR_T([yes]) GLIBCXX_ENABLE_C99([yes]) GLIBCXX_ENABLE_CONCEPT_CHECKS([no]) diff --git a/libstdc++-v3/doc/xml/manual/configure.xml b/libstdc++-v3/doc/xml/manual/configure.xml index 2f558d2..7787bc3 100644 --- a/libstdc++-v3/doc/xml/manual/configure.xml +++ b/libstdc++-v3/doc/xml/manual/configure.xml @@ -278,6 +278,13 @@ </para> </listitem></varlistentry> +<varlistentry><term><code>--enable-new-opnt-no-allocation-retry </code></term> + <listitem><para>This option will cause operator new (std::nothrow) not + to retry allocation if a handler has been set. + The purpose of this is to call the handler just once and return. + </para> + </listitem></varlistentry> + <varlistentry><term><code>--enable-fully-dynamic-string</code></term> <listitem><para>This option enables a special version of basic_string avoiding the optimization that allocates empty objects in static memory. diff --git a/libstdc++-v3/libsupc++/new_opnt.cc b/libstdc++-v3/libsupc++/new_opnt.cc index a9eb465..fac86bc 100644 --- a/libstdc++-v3/libsupc++/new_opnt.cc +++ b/libstdc++-v3/libsupc++/new_opnt.cc @@ -40,7 +40,11 @@ operator new (std::size_t sz, const std::nothrow_t&) _GLIBCXX_USE_NOEXCEPT if (sz == 0) sz = 1; +#ifdef GLIBCXX_ENABLE_NEW_OPNT_NO_ALLOCATION_RETRY + if (__builtin_expect ((p = malloc (sz)) == 0, false)) +#else while (__builtin_expect ((p = malloc (sz)) == 0, false)) +#endif { new_handler handler = std::get_new_handler (); if (! handler)