From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from nikam.ms.mff.cuni.cz (nikam.ms.mff.cuni.cz [195.113.20.16]) by sourceware.org (Postfix) with ESMTPS id 458103858429 for ; Tue, 14 May 2024 11:51:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 458103858429 Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=ucw.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=kam.mff.cuni.cz ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 458103858429 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=195.113.20.16 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1715687473; cv=none; b=Zzf2zimyCwmguDTJMpE5rGxZrkVn/FxX3gM+IFVmy/8dG33XPdDbNArefGIkrmFJZRS6N9OHdXWPQKVlFv+302a1oCvn/lR/LbfUpiLY2gsj+BYu1NK4NW9saCAWMd/g6i47TvPUawHt5DmJtSzzu6n9pS5SFr0qYUJhCfAsvTk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1715687473; c=relaxed/simple; bh=JrGkCuvFMREO8MHchy5yXIFqZwsbbt4uWsAH+JDwTUM=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=xODQ8yJlPjSNTHUk3MfMK7ika4GHO14j3G7UesCHR3lAVgPAsy5WzNzDXWxzN7/hL/tSBDk4Nf8k+zVnjCRzlEFWNdZ0nfsrqhu+Jppmjb/DtpLQPllrzeV88uZ7eLUjhqcufCftK3Ey/hRbmvIfWhVmyPaxSCG/kaSr9+nS6uA= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by nikam.ms.mff.cuni.cz (Postfix, from userid 16202) id 3F28A286F53; Tue, 14 May 2024 13:51:08 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ucw.cz; s=gen1; t=1715687468; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=i/kShGnVGSUgZSyYtTtuTs/4MDTn/i1JJ6M+ccn/i1g=; b=Ttzgoxv3SRpV1Ikp9rOzUZD7KF5AhvC15ZWy0Wv+i2+1kmXxrDI2hT72+/SHqYWqUF7DN2 BlcyReG6zxlMKy2FvBz5FYhLiHBp0AYCH1aMBbp1XUPwF1M1WpKgw3jB0CUqNCDMN9RmCk PxYcmrIODqNmxUeNdrO1yq7riZWQ+6c= Date: Tue, 14 May 2024 13:51:08 +0200 From: Jan Hubicka To: Michal Jires Cc: gcc-patches@gcc.gnu.org Subject: Re: [PATCH 4/7] lto: Implement ltrans cache Message-ID: References: <18cc1c3980551ac1881eea6e78811a629c7baa82.1700222403.git.mjires@suse.cz> <788aa123a8fd4bbfa8a80eda37fbacf38ec78c9b.1700222403.git.mjires@suse.cz> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <788aa123a8fd4bbfa8a80eda37fbacf38ec78c9b.1700222403.git.mjires@suse.cz> X-Spam-Status: No, score=-10.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,JMQ_SPF_NEUTRAL,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_PASS,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: > This patch implements Incremental LTO as ltrans cache. > > The cache is active when directory $GCC_LTRANS_CACHE is specified and exists. > Stored are pairs of ltrans input/output files and input file hash. > File locking is used to allow multiple GCC instances to use to same cache. > > Bootstrapped/regtested on x86_64-pc-linux-gnu > > gcc/ChangeLog: > > * Makefile.in: Add lto-ltrans-cache.o. > * lto-wrapper.cc: Use ltrans cache. > * lto-ltrans-cache.cc: New file. > * lto-ltrans-cache.h: New file. > diff --git a/gcc/lto-ltrans-cache.cc b/gcc/lto-ltrans-cache.cc > new file mode 100644 > index 00000000000..0d43e548fb3 > --- /dev/null > +++ b/gcc/lto-ltrans-cache.cc > @@ -0,0 +1,407 @@ > +/* File caching. > + Copyright (C) 2009-2023 Free Software Foundation, Inc. Probably copyright should be 2023-2024 > +const md5_checksum_t INVALID_CHECKSUM = { Maybe static here? Officially there should be comment before the function. > + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, > +}; > + > +/* Computes checksum for given file, returns INVALID_CHECKSUM if not possible. > + */ comment would look more regular if linebreak is made before possible :) > + > +/* Checks identity of two files byte by byte. */ > +static bool > +files_identical (char const *first_filename, char const *second_filename) > +{ > + FILE *f_first = fopen (first_filename, "rb"); > + if (!f_first) > + return false; > + > + FILE *f_second = fopen (second_filename, "rb"); > + if (!f_second) > + { > + fclose (f_first); > + return false; > + } > + > + bool ret = true; > + > + for (;;) > + { > + int c1, c2; > + c1 = fgetc (f_first); > + c2 = fgetc (f_second); I guess reading by fgetc may get quite ineffecient here. Comparing bigger blocks is probably going to be faster. We could also (incrementally) use mmap where supported. > + > +/* Contructor of cache item. */ > +ltrans_file_cache::item::item (std::string input, std::string output, > + md5_checksum_t input_checksum, uint32_t last_used): Here should be enough whitespace so md5_checksum appears just after ( in line above md5_checksum_t input_checksum, uint32_t last_used): > + input (std::move (input)), output (std::move (output)), > + input_checksum (input_checksum), last_used (last_used) > +{ > + lock = lockfile (this->input + ".lock"); > +} > +/* Destructor of cache item. */ > +ltrans_file_cache::item::~item () > +{ > + lock.unlock (); > +} > + > +/* Reads next cache item from cachedata file. > + Adds `dir/` prefix to filenames. */ > +static ltrans_file_cache::item* > +read_cache_item (FILE* f, const char* dir) > +{ > + md5_checksum_t checksum; > + uint32_t last_used; > + > + if (fread (&checksum, 1, checksum.size (), f) != checksum.size ()) > + return NULL; > + if (fread (&last_used, sizeof (last_used), 1, f) != 1) > + return NULL; > + > + std::vector input (strlen (dir)); > + memcpy (&input[0], dir, input.size ()); > + input.push_back ('/'); Why this is not std::string? > + /* Loads data about previously cached items from cachedata file. > + > + Must be called with creation_lock or deletion_lock held to > + prevent data race. */ > + void > + load_cache (); There should be no newline between type and name. It is there only when defining function (so it is easy to use old-school grep to find where function is defined.) Looks good to me otherwise. Honza