Re: [Suspect SPAM] Re: Should we increase the defaultvacuum_cost_limit? - Mailing list pgsql-hackers

Sorry, I sent a wrong patch. The attached is the right one.

At Mon, 11 Mar 2019 13:57:21 +0100, Julien Rouhaud <rjuju123@gmail.com> wrote in
<CAOBaU_a2tLyonOMJ62=SiDmo84Xo1fy81YA8K=B+=OtTc3sYSQ@mail.gmail.com>
> On Mon, Mar 11, 2019 at 10:03 AM David Rowley
> <david.rowley@2ndquadrant.com> wrote:
> >
> > On Mon, 11 Mar 2019 at 09:58, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > > The second patch is a delta that rounds off to the next smaller unit
> > > if there is one, producing a less noisy result:
> > >
> > > regression=# set work_mem = '30.1GB';
> > > SET
> > > regression=# show work_mem;
> > >  work_mem
> > > ----------
> > >  30822MB
> > > (1 row)
> > >
> > > I'm not sure if that's a good idea or just overthinking the problem.
> > > Thoughts?
> >
> > I don't think you're over thinking it.  I often have to look at such
> > settings and I'm probably not unique in when I glance at 30822MB I can
> > see that's roughly 30GB, whereas when I look at 31562138kB, I'm either
> > counting digits or reaching for a calculator.  This is going to reduce
> > the time it takes for a human to process the pg_settings output, so I
> > think it's a good idea.
> 
> Definitely, rounding up will spare people from wasting time to check
> what's the actual value.

+1. I don't think it overthinking, too.

Anyone who specifies memory size in GB won't care under-MB
fraction. I don't think '0.01GB' is a sane setting but it being
10MB doesn't matter.  However, I don't think that '0.1d' becoming
'2h' is reasonable. "10 times per day" is "rounded" to "12 times
per day" by that.

Is it worth showing values with at most two or three fraction
digits instead of rounding the value on setting? In the attached
PoC patch - instead of the 'roundoff-fractions-harder' patch -
shows values in the shortest exact representation.

work_mem:
   31562138  => '30.1 GB'
   31562137  => '31562137 kB'
   '0.1GB'   => '0.1 GB'
   '0.01GB'  => '0.01 GB'
   '0.001GB' => '1049 kB'

lock_timeout:
   '0.1h'    => '6 min'
   '90 min'  => '90 min'
   '120 min' => '2 h'
   '0.1 d'   => '0.1 d'

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index d374f5372c..21e0807728 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -6005,16 +6005,19 @@ convert_to_base_unit(double value, const char *unit,
 /*
  * Convert an integer value in some base unit to a human-friendly unit.
  *
- * The output unit is chosen so that it's the greatest unit that can represent
- * the value without loss.  For example, if the base unit is GUC_UNIT_KB, 1024
- * is converted to 1 MB, but 1025 is represented as 1025 kB.
+ * The output unit is chosen so that it's the shortest representation that can
+ * represent the value without loss.  For example, if the base unit is
+ * GUC_UNIT_KB, 1024 is converted to 1 MB, but 1025 is represented as 1025 kB.
+ * Also 104858 is converted to '0.1 GB', which is shorter than other
+ * representations.
  */
 static void
 convert_int_from_base_unit(int64 base_value, int base_unit,
-                           int64 *value, const char **unit)
+                           double *value, const char **unit, int *digits)
 {
     const unit_conversion *table;
     int            i;
+    int            len = 0;
 
     *unit = NULL;
 
@@ -6027,17 +6030,49 @@ convert_int_from_base_unit(int64 base_value, int base_unit,
     {
         if (base_unit == table[i].base_unit)
         {
+            const double frac_digits = 2;
+            double rval;
+
             /*
-             * Accept the first conversion that divides the value evenly.  We
-             * assume that the conversions for each base unit are ordered from
-             * greatest unit to the smallest!
+             * Round off the representation at the digit where it is exactly
+             * the same with base_value.
              */
-            if (table[i].multiplier <= 1.0 ||
-                base_value % (int64) table[i].multiplier == 0)
+            rval = (double)base_value / table[i].multiplier;
+            rval = rint(rval * pow(10, frac_digits)) *
+                pow(10, -frac_digits);
+
+            /* Try the unit if it is exact representation */
+            if ((int64)rint(rval * table[i].multiplier) == base_value)
             {
-                *value = (int64) rint(base_value / table[i].multiplier);
-                *unit = table[i].unit;
-                break;
+                int nfrac = 0;
+                int newlen = 1;
+
+                /* Count fraction digits */
+                for (nfrac = 0 ; nfrac < frac_digits ; nfrac++)
+                {
+                    double p = pow(10, nfrac);
+                    if (rval * p - floor(rval * p) < 0.1)
+                        break;
+                }
+
+                /*  Caclculate width of the representation */
+                if (rval >= 1.0)
+                    newlen += floor(log10(rval));
+                newlen += nfrac;
+                if (nfrac > 0)
+                    newlen++; /* for decimal point */
+
+                if (len == 0 || newlen < len)
+                {
+                    *digits = nfrac;
+                    *value = rval;
+                    *unit = table[i].unit;
+                    len = newlen;
+                }
+
+                /* We found the integer representation, exit. */
+                if (nfrac == 0)
+                    break;
             }
         }
     }
@@ -9359,18 +9394,19 @@ _ShowOption(struct config_generic *record, bool use_units)
                      * Use int64 arithmetic to avoid overflows in units
                      * conversion.
                      */
-                    int64        result = *conf->variable;
+                    double        result = *conf->variable;
                     const char *unit;
+                    int            digits = 0;
 
                     if (use_units && result > 0 && (record->flags & GUC_UNIT))
-                        convert_int_from_base_unit(result,
+                        convert_int_from_base_unit(*conf->variable,
                                                    record->flags & GUC_UNIT,
-                                                   &result, &unit);
+                                                   &result, &unit, &digits);
                     else
                         unit = "";
 
-                    snprintf(buffer, sizeof(buffer), INT64_FORMAT "%s",
-                             result, unit);
+                    snprintf(buffer, sizeof(buffer), "%.*f %s",
+                             digits, result, unit);
                     val = buffer;
                 }
             }