From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-3.0 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_NONE shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from mail-ej1-x644.google.com (mail-ej1-x644.google.com [IPv6:2a00:1450:4864:20::644]) (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) (No client certificate requested) by dcvr.yhbt.net (Postfix) with ESMTPS id 9BA5C1F5AE for ; Fri, 31 Jul 2020 14:08:33 +0000 (UTC) Received: by mail-ej1-x644.google.com with SMTP id d6so17724963ejr.5 for ; Fri, 31 Jul 2020 07:08:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=natanson-net.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=4iNSP5uxmCBa1/mNod2W+K0adPUmD6YuJCyqfTGPP6c=; b=KqnNSenxN7Tr+37kKuzWt+KQ1QOyY0E2TLmMXQsF0Af1I763BMQ92MLGhHyV44eE+H ZiSG2nPRiHEbkdf9sgmbZu5aNZBVXc/lUSoqdOnovHxAZ34RYkWNi1TOa9quVgpDAEg0 86IbmCc0uDRiNnBWnDi6mkOnZW6xfYOhTuqGFWyQYzUxDw06oVqJT8YjAM+V4jRnBdA0 n51GWi9PyJ0dV7bGV3qoeRwQYOJNwBScZd/g492SsZVMEWu1DbNnMpvaTzph8PBp5Ajb XL+5uPs0sGxUpcWI1wKDuRxo1B7uVxMjPGrs5ZkEWfQUfbgHDwGS63ARwmd2YSh02Am2 m1JA== 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=4iNSP5uxmCBa1/mNod2W+K0adPUmD6YuJCyqfTGPP6c=; b=VfAqaSAog1bh6i8g586tulIUX0sROOZ96BB4P/nO1fDEOQDZc5eioJQ8pqOaqxOrha 4FewZO2WJeynVEQBWqEjQDfvcB+W7qwiFrCV5lni3Pl7louAbooI+IkTXFZBJ31hl/B6 JwHYfdEslDSxqp6TjNnMpujufOG4OC8HAq4XNUQasEN551FgoSZ3tSGoAjLP1c+af9ZM UBMhD6//c/Isc25OK8/kxl+/VhGLRYlZHmhthDMEy2Fc8Wf0W5hMof5SKgKqch2ebOWR iXQ1s/v85UeYgYtbVsd2TdthamMIGT5WjrRdHDE6EdELJeTZk9idPF7gLI8O2iWW3bRz Y7KA== X-Gm-Message-State: AOAM532YfZ812yDw5CI4BHag+Qplgt8HlSDrXsKCebQ4nMEeLeM7308U vw20a8iz2WAteK2WDS830RUdOiZqlyzeuZW9Tesa4w== X-Google-Smtp-Source: ABdhPJz3w1m/BhsDS4sIJvIBGTAavgp0nFMC06IoQixs1lk/teuPYA+srgPO2OQplWBthIaVS5CFohwTvcYBU4S+B5I= X-Received: by 2002:a17:906:29d5:: with SMTP id y21mr4079114eje.131.1596204512033; Fri, 31 Jul 2020 07:08:32 -0700 (PDT) MIME-Version: 1.0 References: <20200726044647.GA19757@dcvr> <20200727183856.GA26578@dcvr> <20200729092023.GA24100@dcvr> <20200730035421.GA10743@dcvr> In-Reply-To: <20200730035421.GA10743@dcvr> From: Josh Natanson Date: Fri, 31 Jul 2020 10:08:21 -0400 Message-ID: Subject: Re: Clogger request_time formatting To: Eric Wong Cc: clogger-public@yhbt.net Content-Type: text/plain; charset="UTF-8" List-Id: Just sent a patch to you, hopefully in the correct format. If not, please let me know. Thanks again for all your help, it's very much appreciated. - Josh On Wed, Jul 29, 2020 at 11:54 PM Eric Wong wrote: > > Josh Natanson wrote: > > Thanks for the reminder, I hadn't considered the C extension. I took > > a look today, but wasn't able to make any headway - my C is very > > rusty, I'm not sure you would want anything I'd write there anyhow. > > That leaves me a little stuck, with the code working frozen in my > > rails app, but without support in the C layer (or passing tests). I'm > > sure you won't want to incorporate a partial change. If you (or > > someone else) has some time to consider the C side, that would be > > fantastic. If not, I might have to go another route for my logging. > > No worries, below is a proposed patch which adds C support. > > I also made the Ruby format compilation cap power-of-10 to be > <=9 to reduce likelyhood of overflows and non-sensical values. > There's no way to get higher resolution than nanoseconds in > POSIX, and most clocks/kernels are lower resolution, even. > > Arithmetic is not my strong suit, but the C code should be > pretty straightforward if you're familiar with any other > C-like language, Ruby included. > > > Sorry about the email formatting, gmail has ruined me. > > Not just you, 99% of the Internet :< > > > Anyways, can you provide a proper commit message describing the > feature? Thanks. > > ext/clogger_ext/clogger.c | 32 ++++++++++++++++++++++++++++++++ > lib/clogger.rb | 7 ++++++- > 2 files changed, 38 insertions(+), 1 deletion(-) > > diff --git a/ext/clogger_ext/clogger.c b/ext/clogger_ext/clogger.c > index fdc23e3..a316545 100644 > --- a/ext/clogger_ext/clogger.c > +++ b/ext/clogger_ext/clogger.c > @@ -374,12 +374,44 @@ static void append_ts(struct clogger *c, VALUE op, struct timespec *ts) > rb_str_buf_cat(c->log_buf, buf, nr); > } > > +#define NANO_PER_SEC (1000 * 1000 * 1000) > static void append_request_time_fmt(struct clogger *c, VALUE op) > { > struct timespec now; > + unsigned long ipow = NUM2ULONG(rb_ary_entry(op, 3)); > > clock_gettime(hopefully_CLOCK_MONOTONIC, &now); > clock_diff(&now, &c->ts_start); > + if (ipow) { > + struct timespec prev; > + unsigned long adj = 1; > + > + /* > + * n.b. timespec.tv_sec may not be time_t on some platforms, > + * so we use a full timespec struct instead of time_t: > + */ > + prev.tv_sec = now.tv_sec; > + > + do { adj *= 10; } while (--ipow); > + now.tv_sec *= adj; > + now.tv_nsec *= adj; > + if (now.tv_nsec >= NANO_PER_SEC) { > + int64_t add = now.tv_nsec / NANO_PER_SEC; > + > + now.tv_sec += add; > + now.tv_nsec %= NANO_PER_SEC; > + } > + if (now.tv_sec < prev.tv_sec) { /* overflowed */ > + now.tv_nsec = NANO_PER_SEC - 1; > + > + /* > + * some platforms may use unsigned .tv_sec, but > + * they're not worth supporting, so keep unsigned: > + */ > + now.tv_sec = (time_t)(sizeof(now.tv_sec) == 4 ? > + INT_MAX : LONG_MAX); > + } > + } > append_ts(c, op, &now); > } > > diff --git a/lib/clogger.rb b/lib/clogger.rb > index 608909d..8559841 100644 > --- a/lib/clogger.rb > +++ b/lib/clogger.rb > @@ -92,7 +92,12 @@ private > when /\A\$time\{(\d+)\}\z/ > rv << [ OP_TIME, *usec_conv_pair(tok, $1.to_i) ] > when /\A\$request_time\{(\d+),(\d+)\}\z/ > - rv << [ OP_REQUEST_TIME, *usec_conv_pair(tok, $2.to_i), $1.to_i ] > + ipow = $1.to_i > + prec = $2.to_i > + if ipow > 9 # nanosecond precision is the highest POSIX goes > + raise ArgumentError, "#{tok}: too big: #{ipow} (max=9)" > + end > + rv << [ OP_REQUEST_TIME, *usec_conv_pair(tok, prec), ipow ] > when /\A\$request_time\{(\d+)\}\z/ > rv << [ OP_REQUEST_TIME, *usec_conv_pair(tok, $1.to_i), 0 ] > else