From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 61F4A3858D1E for ; Mon, 19 Dec 2022 17:17:29 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 61F4A3858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1671470249; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type; bh=Rv3U4pjucmPi9P+OHKY7uDqTurW4P0CCGrobMRuIlDk=; b=CVUy5n9NKGmSze8hxHsIct3olkouwjihy01W80yYcT6HrHB5ZkOAi5K3yCMwv8c6Sh8Twr j9H3sY2mynGkEytKudQIWcW8ZAyGnfP/7gvDvPe0nROWqAMakwh0Ijq8LE3kXkrI9dJL5Q YpENDA1H3GHEUTqzEbstCKC29T0Bh1o= Received: from mail-yw1-f198.google.com (mail-yw1-f198.google.com [209.85.128.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-626-eyUvybBLMpKyptnfMgGJ1g-1; Mon, 19 Dec 2022 12:17:27 -0500 X-MC-Unique: eyUvybBLMpKyptnfMgGJ1g-1 Received: by mail-yw1-f198.google.com with SMTP id 00721157ae682-3d3769a8103so115878357b3.17 for ; Mon, 19 Dec 2022 09:17:27 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:user-agent:message-id:date:organization:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Rv3U4pjucmPi9P+OHKY7uDqTurW4P0CCGrobMRuIlDk=; b=qSq2dQCtS9hOupPkN8vBgN28Lb2J5Ji+lX4c+Rw6PMoLoACpdIBGLcldDsseXxNJn3 732s/THJPQXDtW7KT7J8UfowiCPzAsoMW1K9VQkUSYW2OcPLm2syMtRXVE+zuhjgHP89 UGpeXMJ7eB6LLajWrEW2p7LYBdtlU7l0rjcm5yf0lcqgRL4a8uGU9F7oOS1JQ0kR2+b2 Ilvhh/kjcY1+7ad4yVW0Q3/je5kwZfH2AJ/Z4vyHdmW5v/ckitcELyjNgWRgrQvHCprr rpdL+LF8JtEduZfmIZFd23W5BAaBOCobT+o4pqjEhAyaEroXgnW7IguwcV63M4shp+ci oROQ== X-Gm-Message-State: AFqh2krTL5/eI7DqdkEgMCSKY5QW+WyipEWGt0fAoJtQ4pcPiZh60Lqt 3HndNEt9TK7NMxpv9LiqITHt+M1sTsTsB7qu/QT6RWA91kmfTTAU5EnYL0dGwzedy9wDzzak+wR gxwRUtKXJXepNXvA0doH7Mz7bCIcPFnlrjkEYhzIvSOsNGCJM/ETJ2H3Op4kXCeSqBXW2 X-Received: by 2002:a05:7500:6d83:b0:ec:9a66:f8c2 with SMTP id ke3-20020a0575006d8300b000ec9a66f8c2mr359527gab.48.1671470246886; Mon, 19 Dec 2022 09:17:26 -0800 (PST) X-Google-Smtp-Source: AMrXdXsDqC+Cc4LbvxeWuqBW42Y4td55Ri8SQgwJ4zPesEPqYLlvd3B3U6s1bKtFacPqg4mmB2mdhA== X-Received: by 2002:a05:7500:6d83:b0:ec:9a66:f8c2 with SMTP id ke3-20020a0575006d8300b000ec9a66f8c2mr359489gab.48.1671470246271; Mon, 19 Dec 2022 09:17:26 -0800 (PST) Received: from localhost ([88.120.130.27]) by smtp.gmail.com with ESMTPSA id c15-20020ae9ed0f000000b006ff8a122a1asm7146817qkg.78.2022.12.19.09.17.25 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 19 Dec 2022 09:17:25 -0800 (PST) Received: by localhost (Postfix, from userid 1000) id CDB26B5649; Mon, 19 Dec 2022 18:17:23 +0100 (CET) From: Dodji Seketeli To: libabigail@sourceware.org Subject: [PATCH, applied] ir: Cache more aggregate type comparison results Organization: Red Hat / France X-Operating-System: CentOS Stream release 9 X-URL: http://www.redhat.com Date: Mon, 19 Dec 2022 18:17:23 +0100 Message-ID: <87fsdbe2jg.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-11.6 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hello, In the aftermath of https://sourceware.org/bugzilla/show_bug.cgi?id=29857, I figured caching comparison results at one place (in a macro) leads to better maintainability. Also, using that macro in the equal() overload for class_decl, union_decl and function_type results in slightly more results of aggregate type comparison being cached during type canonicalization. And that might speed things up. When measuring the impact on the comparison of the bug mentionned above, comparison that takes ~ 43 minutes, we gain 30secs with this patch. We went from: $ time ~/build/tools/abidiff --d1 ghostscript-9.07-31.el7.x86_64/usr/lib/debug --d2 ghostscript-9.52-5.oe1.x86_64/usr/lib/debug/ ghostscript-9.07-31.el7.x86_64/usr/lib64/libgs.so.9 ghostscript-9.52-5.oe1.x86_64/usr/lib64/libgs.so.9 Functions changes summary: 342 Removed, 940 Changed (2084 filtered out), 586 Added functions Variables changes summary: 70 Removed, 432 Changed (333 filtered out), 81 Added variables Function symbols changes summary: 2 Removed, 19 Added function symbols not referenced by debug info Variable symbols changes summary: 0 Removed, 0 Added variable symbol not referenced by debug info (...) real 42m33,633s user 41m54,699s sys 0m1,381s $ to: $ time ~/build/tools/abidiff --d1 ghostscript-9.07-31.el7.x86_64/usr/lib/debug --d2 ghostscript-9.52-5.oe1.x86_64/usr/lib/debug/ ghostscript-9.07-31.el7.x86_64/usr/lib64/libgs.so.9 ghostscript-9.52-5.oe1.x86_64/usr/lib64/libgs.so.9 Functions changes summary: 342 Removed, 940 Changed (2084 filtered out), 586 Added functions Variables changes summary: 70 Removed, 432 Changed (333 filtered out), 81 Added variables Function symbols changes summary: 2 Removed, 19 Added function symbols not referenced by debug info Variable symbols changes summary: 0 Removed, 0 Added variable symbol not referenced by debug info (...) real 42m2,364s user 41m23,911s sys 0m0,326s $ * src/abg-ir.cc (CACHE_AND_RETURN_COMPARISON_RESULT): Define new macro. (equals): In the overloads for function_type, class_decl, and union_decl use the new CACHE_AND_RETURN_COMPARISON_RESULT. Signed-off-by: Dodji Seketeli --- src/abg-ir.cc | 63 ++++++++++++++++----------------------------------- 1 file changed, 20 insertions(+), 43 deletions(-) diff --git a/src/abg-ir.cc b/src/abg-ir.cc index 0c9a32df..08fc3ad0 100644 --- a/src/abg-ir.cc +++ b/src/abg-ir.cc @@ -1081,6 +1081,13 @@ return_comparison_result(T& l, T& r, bool value, ABG_RETURN(value); } +#define CACHE_AND_RETURN_COMPARISON_RESULT(value) \ + do \ + { \ + bool res = return_comparison_result(l, r, value); \ + l.get_environment().priv_->cache_type_comparison_result(l, r, res); \ + return res; \ + } while (false) /// Getter of all types types sorted by their pretty representation. /// /// @return a sorted vector of all types sorted by their pretty @@ -19584,11 +19591,9 @@ function_type::is_variadic() const /// ///@return true if lhs == rhs, false otherwise. bool -equals(const function_type& l, - const function_type& r, - change_kind* k) +equals(const function_type& l, const function_type& r, change_kind* k) { -#define RETURN(value) return return_comparison_result(l, r, value) +#define RETURN(value) CACHE_AND_RETURN_COMPARISON_RESULT(value) RETURN_TRUE_IF_COMPARISON_CYCLE_DETECTED(l, r); @@ -19731,17 +19736,6 @@ equals(const function_type& l, RETURN(result); } - // We are done comparing these two types and we have a full - // understanding of how they might be different, if they are. Let's - // cache the result of this comparison -- in case we are asked in a - // very near future to compare them again. - // - // TODO: If further profiling shows its necessity, maybe we should - // perform this caching also on the earlier return points of this - // function. That would basically mean to redefine the RETURN macro - // to make it perform this caching for us. - l.get_environment().priv_->cache_type_comparison_result(l, r, result); - RETURN(result); #undef RETURN } @@ -21963,9 +21957,6 @@ class_or_union::operator==(const class_or_union& other) const bool equals(const class_or_union& l, const class_or_union& r, change_kind* k) { -#define RETURN(value) return return_comparison_result(l, r, value, \ - /*propagate_canonical_type=*/false); - // if one of the classes is declaration-only, look through it to // get its definition. bool l_is_decl_only = l.get_is_declaration_only(); @@ -22064,6 +22055,15 @@ equals(const class_or_union& l, const class_or_union& r, change_kind* k) if (types_defined_same_linux_kernel_corpus_public(l, r)) return true; + //TODO: Maybe remove this (cycle detection and canonical type + //propagation handling) from here and have it only in the equal + //overload for class_decl and union_decl because this one ( the + //equal overload for class_or_union) is just a sub-routine of these + //two above. +#define RETURN(value) \ + return return_comparison_result(l, r, value, \ + /*propagate_canonical_type=*/false); + RETURN_TRUE_IF_COMPARISON_CYCLE_DETECTED(l, r); mark_types_as_being_compared(l, r); @@ -23621,7 +23621,7 @@ equals(const class_decl& l, const class_decl& r, change_kind* k) mark_types_as_being_compared(l, r); -#define RETURN(value) return return_comparison_result(l, r, value); +#define RETURN(value) CACHE_AND_RETURN_COMPARISON_RESULT(value) // Compare bases. if (l.get_base_specifiers().size() != r.get_base_specifiers().size()) @@ -23735,17 +23735,6 @@ equals(const class_decl& l, const class_decl& r, change_kind* k) } } - // We are done comparing these two types and we have a full - // understanding of how they might be different, if they are. Let's - // cache the result of this comparison -- in case we are asked in a - // very near future to compare them again. - // - // TODO: If further profiling shows its necessity, maybe we should - // perform this caching also on the earlier return points of this - // function. That would basically mean to redefine the RETURN macro - // to make it perform this caching for us. - l.get_environment().priv_->cache_type_comparison_result(l, r, result); - RETURN(result); #undef RETURN } @@ -24746,8 +24735,7 @@ equals(const union_decl& l, const union_decl& r, change_kind* k) return result; } -#define RETURN(value) \ - return return_comparison_result(l, r, value); +#define RETURN(value) CACHE_AND_RETURN_COMPARISON_RESULT(value) bool result = equals(static_cast(l), static_cast(r), @@ -24755,17 +24743,6 @@ equals(const union_decl& l, const union_decl& r, change_kind* k) mark_types_as_being_compared(l, r); - // We are done comparing these two types and we have a full - // understanding of how they might be different, if they are. Let's - // cache the result of this comparison -- in case we are asked in a - // very near future to compare them again. - // - // TODO: If further profiling shows its necessity, maybe we should - // perform this caching also on the earlier return points of this - // function. That would basically mean to redefine the RETURN macro - // to make it perform this caching for us. - l.get_environment().priv_->cache_type_comparison_result(l, r, result); - RETURN(result); } -- 2.31.1 -- Dodji