From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1096 invoked by alias); 20 Dec 2012 16:57:57 -0000 Received: (qmail 1088 invoked by uid 22791); 20 Dec 2012 16:57:57 -0000 X-SWARE-Spam-Status: No, hits=-5.1 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,KHOP_RCVD_TRUST,KHOP_THREADED,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE X-Spam-Check-By: sourceware.org Received: from mail-we0-f177.google.com (HELO mail-we0-f177.google.com) (74.125.82.177) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 20 Dec 2012 16:57:50 +0000 Received: by mail-we0-f177.google.com with SMTP id x48so1659601wey.36 for ; Thu, 20 Dec 2012 08:57:49 -0800 (PST) MIME-Version: 1.0 Received: by 10.180.99.129 with SMTP id eq1mr10954860wib.30.1356022669009; Thu, 20 Dec 2012 08:57:49 -0800 (PST) Received: by 10.216.116.67 with HTTP; Thu, 20 Dec 2012 08:57:48 -0800 (PST) In-Reply-To: <20121220162054.GA26643@atrey.karlin.mff.cuni.cz> References: <20121219200828.73DB9106927@rong.mtv.corp.google.com> <20121220162054.GA26643@atrey.karlin.mff.cuni.cz> Date: Thu, 20 Dec 2012 16:57:00 -0000 Message-ID: Subject: Re: [google 4.7] atomic update of profile counters (issue6965050) From: Andrew Pinski To: Jan Hubicka Cc: Rong Xu , GCC Patches , David Li , reply@codereview.appspotmail.com Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2012-12/txt/msg01256.txt.bz2 On Thu, Dec 20, 2012 at 8:20 AM, Jan Hubicka wrote: >> On Wed, Dec 19, 2012 at 4:29 PM, Andrew Pinski wrote: >> > >> > On Wed, Dec 19, 2012 at 12:08 PM, Rong Xu wrote: >> > > Hi, >> > > >> > > This patch adds the supprot of atomic update the profile counters. >> > > Tested with google internal benchmarks and fdo kernel build. >> > >> > I think you should use the __atomic_ functions instead of __sync_ >> > functions as they allow better performance for simple counters as you >> > can use __ATOMIC_RELAXED. >> >> You are right. I think __ATOMIC_RELAXED should be OK here. >> Thanks for the suggestion. >> >> > >> > And this would be useful for the trunk also. I was going to implement >> > this exact thing this week but some other important stuff came up. >> >> I'll post trunk patch later. > > Yes, I like that patch, too. Even if the costs are quite high (and this is why > atomic updates was sort of voted down in the past) the alternative of using TLS > has problems with too-much per-thread memory. Actually sometimes (on some processors) atomic increments are cheaper than doing a regular incremental. Mainly because there is an instruction which can handle it in the L2 cache rather than populating the L1. Octeon is one such processor where this is true. Thanks, Andrew Pinski > > While there are even more alternatives, like recording the changes and > commmiting them in blocks (say at function return), I guess some solution is > better than no solution. > > Thanks, > Honza