From owner-svn-src-head@freebsd.org Thu Nov 19 00:37:58 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 082174762BC; Thu, 19 Nov 2020 00:37:58 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: from mail-wr1-x429.google.com (mail-wr1-x429.google.com [IPv6:2a00:1450:4864:20::429]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Cc1456fRdz4shC; Thu, 19 Nov 2020 00:37:57 +0000 (UTC) (envelope-from mjguzik@gmail.com) Received: by mail-wr1-x429.google.com with SMTP id k2so4668970wrx.2; Wed, 18 Nov 2020 16:37:57 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=C9z7TbRsPRCO8jmyzl+/J+k/UrJWz7WMf25SE8al9Xk=; b=VqPIEyIkcENFgidVXbqW9FUdpfAWwa3aiGmRPiSNV6eNg1/mN3mfCA3bWx4wvxeahD u5ywOcIowRcUC7L6v12KOieAPpIr8YacFCKHi3n671Fo8gKdx5+5rL3KG3UbY3UGYUOp fqyS4ibUtsP2tsCH4Qeoh6uG6U+iT4MJX0yw6H2WF773bx5UJ5a0BuUiJYEgrJJa66tW dJKeQEZs2x59zQS1/d6cpYO0eXyE2soSKtDpKp5tP2KLL4huDkVyc7JiIR2hDF5es4c/ WGjulKMYHk1Ku5kwxW10v1BOVghzWdsdWVJFTNHzdL4n/YBqCUyXJP8GQ3cIDYI0ofDI DB3w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=C9z7TbRsPRCO8jmyzl+/J+k/UrJWz7WMf25SE8al9Xk=; b=DULgMaAbLazmIc6AJQQWFpwtFN+Mdoq5HCYAhxV+TgtAMoe24rdR0ZWbuj8v5V0IjK hJBWwPVjZAGYt0TCihIpAB52V9Qafotx1MzOEgFQSyI0KuIZI4Pes/x3u7nL0gTy0aow JoHs2DLkJQMga6O3GQ8jrkW6OFoMqO8uC8gBGXyXWaFrzajXo4jBoLyNUr5/aipfxC7D +v5vktdWVr/yk84oyCgztHaJ9z71xX8IYJP+Rr145By0psn9FGPQp/o/SAv50xB5jrc5 a80OZBJ4Z4j4HV9034oGm4lTAa/Zg/x/XINH6bFz51Qd/jcTeTmRFdQZqx5GSWU1AS2o aFDg== X-Gm-Message-State: AOAM531RtrVoT0m+bCI375rO7+FKpZlXYTOxkLb7dL7QzJTlJfiW7ekr N7Qdb8as8vgCkp8BWaLMYCX8/DiF4ymeoHpb2R24iXibFlI= X-Google-Smtp-Source: ABdhPJyGx7kjwmjpuWWO1AnfWhLVXze/Jx5lqn3Y8MKH4sTbQv7XX2r1hprJPXrCmOPsYsRTkwmKVH/b5VawdytWlMo= X-Received: by 2002:a5d:5146:: with SMTP id u6mr7763612wrt.66.1605746275469; Wed, 18 Nov 2020 16:37:55 -0800 (PST) MIME-Version: 1.0 Received: by 2002:adf:dec7:0:0:0:0:0 with HTTP; Wed, 18 Nov 2020 16:37:54 -0800 (PST) In-Reply-To: <81fd2ce6-b5b1-7f05-c575-6b233e78b739@freebsd.org> References: <202011181944.0AIJiUU3003699@repo.freebsd.org> <81fd2ce6-b5b1-7f05-c575-6b233e78b739@freebsd.org> From: Mateusz Guzik Date: Thu, 19 Nov 2020 01:37:54 +0100 Message-ID: Subject: Re: svn commit: r367813 - head/lib/libutil To: Stefan Esser Cc: Jessica Clarke , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 4Cc1456fRdz4shC X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Nov 2020 00:37:58 -0000 On 11/19/20, Stefan Esser wrote: > Am 18.11.20 um 23:39 schrieb Jessica Clarke: >> On 18 Nov 2020, at 22:32, Stefan Esser wrote: >>> >>> Am 18.11.20 um 22:40 schrieb Mateusz Guzik: >>>>> +{ >>>>> + static const int localbase_oid[2] = {CTL_USER, USER_LOCALBASE}; >>>> There is no use for this to be static. >>> >>> Why not? This makes it part of the constant initialized data of >>> the library, which is more efficient under run-time and memory >>> space aspects than initializing them on the stack. >>> >>> What do I miss? >> >> What is more efficient is not so clear-cut, it depends on things like >> the architecture, microarchitecture and ABI. Allocating a small buffer >> on the stack is extremely cheap (the memory will almost certainly be in >> the L1 cache), whereas globally-allocated storage is less likely to be >> in the cache due to being spread out, and on some architecture/ABI >> combinations will incur additional indirection through the GOT. Also, 8 >> bytes of additional stack use is lost in the noise. > > The memory latency of the extra access to the constant will be hidden in > the noise. The data will probably be in a page that has already been > accessed (so no TLB load penalty) and modern CPUs will be able to deal > with the cache miss (if any, because the cache line may already be > loaded depending on other data near-by). > > Yes, I do agree that a stack local variable could have been used and > it could have been created with little effort by a modern multi-issue > CPU. The code size would have been larger, though, by some 10 to 20 > bytes, I'd assume - but I doubt a single path through this code is > measurable, much less observable in practice. > > We are talking about nano-seconds here (even if the cache line did > not contain the constant data, it would probably be accessed just a > few instructions further down and incur the same latency then). > > I have followed CPU development over more than 45 years and know many > architectures and their specifics, but the time were I have programmed > drivers in assembly and counted instruction cycles is long gone. > > This is nitpicking at a level that I do not want to continue. I'm not > opposed to achieving efficiency where it is relevant. This function is > providing useful functionality and I do not mind a wasted microsecond, > it is not relevant here. (This was different if it was wasted within > a tight loop - but it is not, it is typically called once if at all). > > Feel free to replace my code with your implementation, if you think it > is not acceptable as written by me. > > I just wanted to provide an implementation of this functionality to > be used in a number of programs where other developers had expressed > interest in such a feature (and one of these programs has been worked > on by me in recent weeks, so I'm now able to make use of it myself). > The entire localbase saga is getting way out of hand. To address this e-mail and things you wrote in another reply, my comlaints are very simple and are not getting less valid for not being raised sooner. I just was not watching any of this until recent fallout. For the change at hand, there has to be a reason to use a static symbol. Standard example is catching otherwise expensive to obtain data. For that the static localbase pointer makes perfect sense, while the static lookup array does not have any justification that I see. Bringing cache, TLB or whatever microarchitectural details into the discussion is beyond not warranted. More importantly though the commit comes with a self-confessed memory leak and is a show stopper. That said, I'll see about patching this up. -- Mateusz Guzik