From: "Matthias Männich" <maennich@google.com>
To: Dodji Seketeli <dodji@seketeli.org>
Cc: Giuliano Procida <gprocida@google.com>,
Android Kernel Team <kernel-team@android.com>,
Giuliano Procida via Libabigail <libabigail@sourceware.org>
Subject: Re: [PATCH, applied] Consistently use std::unique_ptr for private implementations (pimpl)
Date: Fri, 16 Jul 2021 10:48:50 +0100 [thread overview]
Message-ID: <CAJFNNnoZM=vLmh8MXDKG-X1CbCtWUJjMBu7iKX1arjaDYda2Eg@mail.gmail.com> (raw)
In-Reply-To: <87zgumei54.fsf@seketeli.org>
On Fri, Jul 16, 2021 at 10:13 AM Dodji Seketeli <dodji@seketeli.org> wrote:
>
> Hello,
>
> Matthias Maennich <maennich@google.com> a écrit:
>
> > In the absence of non-refcounting smart pointers before C++11,
> > std::shared_ptr was commonly used instead. Having bumped the standard to
> > C++11, allows us to use std::unique_ptr consistently avoiding any costs
> > involved with shared_ptr ref counting. Hence do that and add default
> > virtual destructors where required.
>
> Thanks for doing this!
>
> So, after testing on an old AMD machine, here is the difference in the
> time taken (as reported by the 'time' command) by "make check" between
> the master branch without the patch, and with the patch.
>
> Without the patch:
>
> real 3m41,260s
> user 10m44,672s
> sys 1m27,097s
>
> With the patch:
>
> real 2m7,102s
> user 11m48,024s
> sys 1m18,589s
>
> Pretty neat isn't it? :-)
Oh wow! That is indeed neat! Guiliano was benchmarking the change and
came to an improvement there as well. The effect was not as strong,
though:
abidw: 1:43 or 1:47
abidiff: 1:31
after:
abidw: 1:42 or 1:45
abidiff: 1:30
Cheers,
Matthias
>
> So thanks a lot for that!
>
> >
> > * include/abg-comparison.h (diff_maps): use unique_ptr for priv_
> > (diff_context): Likewise.
> > (diff_traversable_base): Likewise.
> > (type_diff_base): Likewise.
> > (decl_diff_base): Likewise.
> > (distinct_diff): Likewise.
> > (var_diff): Likewise.
> > (pointer_diff): Likewise.
> > (reference_diff): Likewise.
> > (array_diff): Likewise.
> > (qualified_type_diff): Likewise.
> > (enum_diff): Likewise.
> > (class_or_union_diff): Likewise.
> > (class_diff): Likewise.
> > (base_diff): Likewise.
> > (scope_diff): Likewise.
> > (fn_parm_diff): Likewise.
> > (function_type_diff): Likewise.
> > (function_decl_diff): Likewise.
> > (typedef_diff): Likewise.
> > (translation_unit_diff): Likewise.
> > (diff_stats): Likewise.
> > (diff_node_visitor): Likewise.
> > * include/abg-corpus.h (corpus): Likewise.
> > (exported_decls_builder): Likewise.
> > (corpus_group): Likewise.
> > * include/abg-ini.h (property): Likewise.
> > (property_value): Likewise.
> > (string_property_value): Likewise.
> > (list_property_value): Likewise.
> > (tuple_property_value): Likewise.
> > (simple_property): Likewise.
> > (list_property): Likewise.
> > (tuple_property): Likewise.
> > (config): Likewise.
> > (section): Likewise.
> > (function_call_expr): Likewise.
> > * include/abg-interned-str.h (interned_string_pool): Likewise.
> > * include/abg-ir.h (environment): Likewise.
> > (location_manager): Likewise.
> > (type_maps): Likewise.
> > (translation_unit): Likewise.
> > (elf_symbol::version): Likewise.
> > (type_or_decl_base): Likewise.
> > (scope_decl): Likewise.
> > (qualified_type_def): Likewise.
> > (pointer_type_def): Likewise.
> > (array_type_def): Likewise.
> > (subrange_type): Likewise.
> > (enum_type_decl): Likewise.
> > (enum_type_decl::enumerator): Likewise.
> > (typedef_decl): Likewise.
> > (dm_context_rel): Likewise.
> > (var_decl): Likewise.
> > (function_decl::parameter): Likewise.
> > (function_type): Likewise.
> > (method_type): Likewise.
> > (template_decl): Likewise.
> > (template_parameter): Likewise.
> > (type_tparameter): Likewise.
> > (non_type_tparameter): Likewise.
> > (template_tparameter): Likewise.
> > (type_composition): Likewise.
> > (function_tdecl): Likewise.
> > (class_tdecl): Likewise.
> > (class_decl::base_spec): Likewise.
> > (ir_node_visitor): Likewise.
> > * include/abg-suppression.h (suppression_base): Likewise.
> > (type_suppression::insertion_range): Likewise.
> > (type_suppression::insertion_range::boundary): Likewise.
> > (type_suppression::insertion_range::integer_boundary): Likewise.
> > (type_suppression::insertion_range::fn_call_expr_boundary): Likewise.
> > (function_suppression): Likewise.
> > (function_suppression::parameter_spec): Likewise.
> > (file_suppression): Likewise.
> > * include/abg-tools-utils.h (temp_file): Likewise.
> > (timer): Likewise.
> > * include/abg-traverse.h (traversable_base): Likewise.
> > * include/abg-workers.h (queue): Likewise.
> > * src/abg-comparison.cc (diff_context): add default destructor.
> > (diff_maps): Likewise.
> > (corpus_diff): Likewise.
> > (diff_node_visitor): Likewise.
> > (class_or_union_diff::get_priv): adjust return type.
> > (class_diff::get_priv): adjust return type.
> > * src/abg-corpus.cc (corpus): add default destructor.
> > * src/abg-ir.cc (location_manager): Likewise.
> > (type_maps): Likewise.
> > (elf_symbol::version): Likewise.
> > (array_type_def::subrange_type): Likewise.
> > (enum_type_decl::enumerator): Likewise.
> > (function_decl::parameter): Likewise.
> > (class_decl::base_spec): Likewise.
> > (ir_node_visitor): Likewise.
> >
> > Signed-off-by: Matthias Maennich <maennich@google.com>
>
> Applied to master. Thanks!
>
> [...]
>
> Cheers,
>
> --
> Dodji
next prev parent reply other threads:[~2021-07-16 9:49 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-05 9:45 [PATCH] " Matthias Maennich
2021-07-16 9:13 ` [PATCH, applied] " Dodji Seketeli
2021-07-16 9:48 ` Matthias Männich [this message]
2021-07-16 9:56 ` Giuliano Procida
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAJFNNnoZM=vLmh8MXDKG-X1CbCtWUJjMBu7iKX1arjaDYda2Eg@mail.gmail.com' \
--to=maennich@google.com \
--cc=dodji@seketeli.org \
--cc=gprocida@google.com \
--cc=kernel-team@android.com \
--cc=libabigail@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).