From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x32f.google.com (mail-wm1-x32f.google.com [IPv6:2a00:1450:4864:20::32f]) by sourceware.org (Postfix) with ESMTPS id C968C3857830 for ; Fri, 16 Jul 2021 09:56:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C968C3857830 Received: by mail-wm1-x32f.google.com with SMTP id a5-20020a7bc1c50000b02901e3bbe0939bso5488697wmj.0 for ; Fri, 16 Jul 2021 02:56:38 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=/lAyGak0vhZDGlPXRlm8tlVRWrjVAisu0dswhZ6EV+4=; b=cq3UDE+70DHcKjZgs8jBEQSmUjgr0A5ZgbYdbFWTybJ8jFv7ts1CI7SjbixyL1TpBV gf0IwDqw8ATI8IHRuT+ee92UFQLvzHnJI7DV4Nwwkwgy1PBjve2O6i1oAmKaxHX3Jzsr ceshEWfsmkUGSl8jt9xIhSV8mYoBXuvVP7CkEHcGIp1zyRGc+5PBMv1jWPnJlxQHq+o8 vDC7xEXZFmmp6re547yhfVaX+ZMyJyLN/wdSfTU2m+TRADbBAxy1Uj2g2XbTaLX8dR/3 5zRA7GbRGcSI6w+kueljVXjpVtLnSZ1UZ0VQrql85iDxaxtrgKGumuT7KrssARP8R4R9 trmA== X-Gm-Message-State: AOAM531tyGAnklkVAriZV6sOqQoDKv715oBYOZqS1FOU0FMv/VSPXZJK /VTnKxy7J2voNUvU94pjt3hHHeNJPR6HsMQB4H55RQ== X-Google-Smtp-Source: ABdhPJyYGh0/IqMrgSmgaKGdkwjOaCdxg4HQ/fF2WBN5sZ3jfioFn3cuAsKaZG715Z3WgDAmjMFYvDOdfhuupwXZZVc= X-Received: by 2002:a7b:cb53:: with SMTP id v19mr9274946wmj.127.1626429397733; Fri, 16 Jul 2021 02:56:37 -0700 (PDT) MIME-Version: 1.0 References: <20210705094529.2485114-1-maennich@google.com> <87zgumei54.fsf@seketeli.org> In-Reply-To: From: Giuliano Procida Date: Fri, 16 Jul 2021 10:56:26 +0100 Message-ID: Subject: Re: [PATCH, applied] Consistently use std::unique_ptr for private implementations (pimpl) To: =?UTF-8?Q?Matthias_M=C3=A4nnich?= Cc: Dodji Seketeli , Android Kernel Team , Giuliano Procida via Libabigail X-Spam-Status: No, score=-22.4 required=5.0 tests=BAYES_00, DKIMWL_WL_MED, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, ENV_AND_HDR_SPF_MATCH, HTML_MESSAGE, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, USER_IN_DEF_DKIM_WL, USER_IN_DEF_SPF_WL autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: libabigail@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Mailing list of the Libabigail project List-Unsubscribe: , List-Archive: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 16 Jul 2021 09:56:41 -0000 I was benchmarking the kernel ABI use case. So it looks like perhaps the more complex C++ ABIs really benefit from this change. Matthias, I'm glad we found the performance improvement after all! On Fri, 16 Jul 2021, 10:49 Matthias M=C3=A4nnich, wro= te: > On Fri, Jul 16, 2021 at 10:13 AM Dodji Seketeli > wrote: > > > > Hello, > > > > Matthias Maennich a =C3=A9crit: > > > > > 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 cos= ts > > > 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 > > > > Applied to master. Thanks! > > > > [...] > > > > Cheers, > > > > -- > > Dodji >