Re: Re: Math.minmax

classic Classic list List threaded Threaded
23 messages Options
12
Reply | Threaded
Open this post in threaded view
|

Re: Re: Math.minmax

Xavier Stouder

Doesn’t performance matter?


_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Re: Math.minmax

Xavier Stouder
As adviced by annevk on IRC, I'm gonna explains the results better.

Minmax is more efficient that a min and a max : you loop one time
instance of two. Seems logic.

To not destructurate the array of arguments is of course better because
at the end, by destructurating you're gonna work on an arguments array.

I actually made the proposal this way to fit with existing min and
max (taking arguments separately). Gonna edit the proposal to accept an
array directly.

Don't know what Boris mean when he talks about recreation bits

Using both min and max is a regular use case of these functions, so x2
improvement on this is an existing need. I mentioned an example of use
above.
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Math.minmax

Boris Zbarsky
On 10/2/17 7:10 AM, Xavier Stouder wrote:
> Don't know what Boris mean when he talks about recreation bits

Fwiw, it looks like the code at
https://esbench.com/bench/595c1b1899634800a03488b9 does not have the
array recreation bits (function whatever(...args)) that earlier
benchmarks for this had.

-Boris
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Math.minmax

Naveen Chawla
I would just use reduce for this. Reason: I think any multi var result format is a little messy. I find it better to let the dev decide on the result format, e.g.:

```js
const minMax =
     array.reduce(
         (accumulator, currentValue)=>{
              return {
                  min: Math.min(currentValue, accumulator.min),
                  max: Math.max(currentValue, accumulator.max)
              }
         },
         {
             min: Infinity,
             max: -Infinity
         }
     )
```

Good thing is, this can easily be refactored to accept arrays with objects that contain the values, instead of just an array of numbers, as well as the ability to calculator other accumulated values (e.g. mean average etc.) in the same call.

Do let me know if you think I'm missing the point

On Mon, 2 Oct 2017 at 19:16 Boris Zbarsky <[hidden email]> wrote:
On 10/2/17 7:10 AM, Xavier Stouder wrote:
> Don't know what Boris mean when he talks about recreation bits

Fwiw, it looks like the code at
https://esbench.com/bench/595c1b1899634800a03488b9 does not have the
array recreation bits (function whatever(...args)) that earlier
benchmarks for this had.

-Boris
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss

_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Math.minmax

Michael Rosefield
I think your approach is fine, but just to be that guy I'll condense it some more (could be output as a hash but, if we're going to condense, well...):

```js
const minMax = arr => arr.reduce(
   ([ min, max ], curr) => [ Math.min(curr, min), Math.max(curr, max) ],
   [ Infinity, -Infinity ]
  )

On Mon, 2 Oct 2017 at 16:34 Naveen Chawla <[hidden email]> wrote:
I would just use reduce for this. Reason: I think any multi var result format is a little messy. I find it better to let the dev decide on the result format, e.g.:

```js
const minMax =
     array.reduce(
         (accumulator, currentValue)=>{
              return {
                  min: Math.min(currentValue, accumulator.min),
                  max: Math.max(currentValue, accumulator.max)
              }
         },
         {
             min: Infinity,
             max: -Infinity
         }
     )
```

Good thing is, this can easily be refactored to accept arrays with objects that contain the values, instead of just an array of numbers, as well as the ability to calculator other accumulated values (e.g. mean average etc.) in the same call.

Do let me know if you think I'm missing the point

On Mon, 2 Oct 2017 at 19:16 Boris Zbarsky <[hidden email]> wrote:
On 10/2/17 7:10 AM, Xavier Stouder wrote:
> Don't know what Boris mean when he talks about recreation bits

Fwiw, it looks like the code at
https://esbench.com/bench/595c1b1899634800a03488b9 does not have the
array recreation bits (function whatever(...args)) that earlier
benchmarks for this had.

-Boris
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss

_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Math.minmax

Michael Rosefield
Oh, and of course if I'm going to be that guy I should immediately post a slightly better version just to annoy people more:

```js
const minMax = (arr=[]) => arr.reduce(
   ([ min=Infinity, max=-Infinity ], curr) => [ Math.min(curr, min), Math.max(curr, max) ], []
  )

On Mon, 2 Oct 2017 at 16:44 Michael Rosefield <[hidden email]> wrote:
I think your approach is fine, but just to be that guy I'll condense it some more (could be output as a hash but, if we're going to condense, well...):

```js
const minMax = arr => arr.reduce(
   ([ min, max ], curr) => [ Math.min(curr, min), Math.max(curr, max) ],
   [ Infinity, -Infinity ]
  )

On Mon, 2 Oct 2017 at 16:34 Naveen Chawla <[hidden email]> wrote:
I would just use reduce for this. Reason: I think any multi var result format is a little messy. I find it better to let the dev decide on the result format, e.g.:

```js
const minMax =
     array.reduce(
         (accumulator, currentValue)=>{
              return {
                  min: Math.min(currentValue, accumulator.min),
                  max: Math.max(currentValue, accumulator.max)
              }
         },
         {
             min: Infinity,
             max: -Infinity
         }
     )
```

Good thing is, this can easily be refactored to accept arrays with objects that contain the values, instead of just an array of numbers, as well as the ability to calculator other accumulated values (e.g. mean average etc.) in the same call.

Do let me know if you think I'm missing the point

On Mon, 2 Oct 2017 at 19:16 Boris Zbarsky <[hidden email]> wrote:
On 10/2/17 7:10 AM, Xavier Stouder wrote:
> Don't know what Boris mean when he talks about recreation bits

Fwiw, it looks like the code at
https://esbench.com/bench/595c1b1899634800a03488b9 does not have the
array recreation bits (function whatever(...args)) that earlier
benchmarks for this had.

-Boris
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss

_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Re: Math.minmax

Xavier Stouder
In reply to this post by Naveen Chawla
No problem Boris, I edited this times a long time ago.

Naveen, you missed he point. In fact, I just added your code the
benchmark (link aboce) and it has catastrophic performances.
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Re: Math.minmax

Xavier Stouder
In reply to this post by Michael Rosefield
Same for Michael. Useless to not use a reducer instead of Math.min and
Math.max if it has worth performance.

Just to be clear, the fact is that your function approximately costs:
Math.min: one loop over the array
Math.max: one loop over the array
Math.minMax: one loop over the array

Math.minMax do in one pass what Math.min and Math.max do in two passes.
That's the key point.
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Re: Math.minmax

J Decker
In reply to this post by Xavier Stouder


On Mon, Oct 2, 2017 at 8:49 AM, Xavier Stouder <[hidden email]> wrote:
No problem Boris, I edited this times a long time ago.

Naveen, you missed he point. In fact, I just added your code the
benchmark (link aboce) and it has catastrophic performances.

Ya, that's a lot of array creations, not to mention the callback in the reduce()...

```js
const minMax = (arr=[]) => { let result=[Infinity, -Infinity]; for( let i=0;i < arr.length;i++ ) {
  result[0] = 
Math.min(arr[i], result[0]);
  result[1] = Math.max(arr[i], result[1]);
 }
 return result;
}
```
although I suspect this will be faster...
```js
const minMax = (arr=[]) => { let result=[Infinity, -Infinity]; for( let i=0;i < arr.length;i++ ) { 
  result[0] = 
arr[i] < result[0] ? arr[i]:result[0];
  result[1] = arr[i] > result[1]? arr[i]: result[1];
 }
 return result;
}
```

 
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss


_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Math.minmax

kai zhu
In reply to this post by Xavier Stouder
hi xavier, i've attached some screenshots to explain why Math.minmax has little performance impact on a webapp as a whole.

here's a screenshot showing it takes 1) 30ms to run a single Math.min() operation one million times and 2) 50ms to run both Math.min() and Math.max() one million times.  assuming the hypothetical Math.minmax() ideally runs as fast as Math.min(), it will save you 20ms for every one million operations.

```javascript
/*jslint node: true*/
/*globals Float64Array*/
'use strict';

var ii, list1, list2, result, time;

list1 = new Float64Array(1000000);
list2 = new Float64Array(1000000);
for (ii = 0; ii < 1000000; ii += 1) {
    list1[ii] = Math.random();
    list2[ii] = Math.random();
}
// pre-trial run to get vm-optimization shenanigans out of the way
for (ii = 0; ii < 1000000; ii += 1) {
    result = Math.min(list1[ii], list2[ii]);
    result = Math.max(list1[ii], list2[ii]);
}
time = Date.now();
for (ii = 0; ii < 1000000; ii += 1) {
    result = Math.min(list1[ii], list2[ii]);
}
console.log((Date.now() - time) + 'ms - 1,000,000 min calculations');
time = Date.now();
for (ii = 0; ii < 1000000; ii += 1) {
    result = Math.min(list1[ii], list2[ii]);
    result = Math.max(list1[ii], list2[ii]);
}
console.log((Date.now() - time) + 'ms - 1,000,000 min and max calculations');
```
here's a screenshot showing how long it takes to load a google-search webpage (arguably the fastest-loading real-use webpage in the world), under ideal conditions with pre-caching and all.  as you can see, it takes on average ~700ms to load the entire page with caching.

think about it.  the performance savings of 20ms for one million Math.minmax operations is at best gonna improve your webapp performance by less than 3% (assuming a 700ms load-time is all the webapp does).

On Oct 2, 2017, at 8:10 PM, Xavier Stouder <[hidden email]> wrote:

As adviced by annevk on IRC, I'm gonna explains the results better.

Minmax is more efficient that a min and a max : you loop one time
instance of two. Seems logic.

To not destructurate the array of arguments is of course better because
at the end, by destructurating you're gonna work on an arguments array.

I actually made the proposal this way to fit with existing min and
max (taking arguments separately). Gonna edit the proposal to accept an
array directly.

Don't know what Boris mean when he talks about recreation bits

Using both min and max is a regular use case of these functions, so x2
improvement on this is an existing need. I mentioned an example of use
above.
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss


_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Re: Math.minmax

Michael Rosefield
In reply to this post by Xavier Stouder
Ah yes, woods, I was looking at the trees.

Still, to carry on with the concise style:

```js
const minMax = (arr=[]) => arr.reduce(
  ([min=Infinity, max=-Infinity], curr) => [ min < curr ? min : curr, max > curr ? max : curr ], []
)
 
```

On Mon, 2 Oct 2017 at 16:55 Xavier Stouder <[hidden email]> wrote:
Same for Michael. Useless to not use a reducer instead of Math.min and
Math.max if it has worth performance.

Just to be clear, the fact is that your function approximately costs:
Math.min: one loop over the array
Math.max: one loop over the array
Math.minMax: one loop over the array

Math.minMax do in one pass what Math.min and Math.max do in two passes.
That's the key point.
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss

_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Re: Math.minmax

Xavier Stouder
In reply to this post by kai zhu
JDecker: Just added your solution on the benchmark, it beats every
others solution and it's a elegant solution.

Kai Zhu: We can't see the screenshot. But please take in consideration
that it's been a long time that ECMAScript isn't only used in webapp,
and that some of applications using it can eat more than a million
numbers.
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Re: Math.minmax

Ben Newman
Taking a step back from the details of this proposal, I have some thoughts about why it seems to be struggling to find support.

In no particular order, I would say this proposal
  • relies on microbenchmarks, which can be misleading
  • disregards Amdahl's Law, by pretending that real-world JS CPU usage is commonly/ever dominated by min/max computations
  • replaces two O(n) loops with another O(n) loop that does slightly more work on each iteration, resulting in no complexity improvement, and a fairly modest (< 2x) constant factor improvement
  • doesn't seem to provide usability/learnability improvements for any particular group of JS developers (for example, novice programmers)
  • doesn't seem to prevent any common bugs in JS code
As a member of TC39, I regret that we have not provided a clearer set of criteria for what it takes to get a new function into the standard library. While I can't speak for the committee as a whole, my suspicion is that this proposal is unlikely to meet that standard. It's a fine idea, but so are many other functions that you can implement in a normal (non-standard) library.

I would also challenge the committee to think about (or link to!) any concrete written criteria that someone with an idea for a proposal could use to assess its chances of acceptance. Imagine how much time we could save!

Ben

On Mon, Oct 2, 2017 at 12:12 PM Xavier Stouder <[hidden email]> wrote:
JDecker: Just added your solution on the benchmark, it beats every
others solution and it's a elegant solution.

Kai Zhu: We can't see the screenshot. But please take in consideration
that it's been a long time that ECMAScript isn't only used in webapp,
and that some of applications using it can eat more than a million
numbers.
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss

_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Re: Math.minmax

Naveen Chawla
In reply to this post by Xavier Stouder
I would be curious about the reduce version that doesn't create a new object/array on every iteration:

```js
const minMax =
     array.reduce(
         (accumulator, currentValue)=>{
              accumulator.min = Math.min(currentValue, accumulator.min);
              accumulator.max = Math.max(currentValue, accumulator.max);
              return accumulator;
         },
         {
             min: Infinity,
             max: -Infinity
         }
     )
```

If you could let me know the relative performance of this in the benchmark it would be great.

On Mon, 2 Oct 2017 at 21:43 Xavier Stouder <[hidden email]> wrote:
JDecker: Just added your solution on the benchmark, it beats every
others solution and it's a elegant solution.

Kai Zhu: We can't see the screenshot. But please take in consideration
that it's been a long time that ECMAScript isn't only used in webapp,
and that some of applications using it can eat more than a million
numbers.
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss

_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Re: Math.minmax

Andrea Giammarchi-2
can I ask why nobody is using Math.min/max signature at their full potentials ?

```js
const [min, max] = [
  Math.min.apply(null, array),
  Math.max.apply(null, array)
];
```

also, why are benchmarks using `Date.now()` instead of the more accurate `performance.now()` or the dedicated `console.time('bench') / console.timeEnd('bench')` utility?



On Mon, Oct 2, 2017 at 3:00 PM, Naveen Chawla <[hidden email]> wrote:
I would be curious about the reduce version that doesn't create a new object/array on every iteration:

```js
const minMax =
     array.reduce(
         (accumulator, currentValue)=>{
              accumulator.min = Math.min(currentValue, accumulator.min);
              accumulator.max = Math.max(currentValue, accumulator.max);
              return accumulator;
         },
         {
             min: Infinity,
             max: -Infinity
         }
     )
```

If you could let me know the relative performance of this in the benchmark it would be great.

On Mon, 2 Oct 2017 at 21:43 Xavier Stouder <[hidden email]> wrote:
JDecker: Just added your solution on the benchmark, it beats every
others solution and it's a elegant solution.

Kai Zhu: We can't see the screenshot. But please take in consideration
that it's been a long time that ECMAScript isn't only used in webapp,
and that some of applications using it can eat more than a million
numbers.
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss

_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss



_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Math.minmax

Allen Wirfs-Brock
In reply to this post by Ben Newman
Another important characteristic we look for in proposals is orthogonality: https://en.wikipedia.org/wiki/Orthogonality#Computer_science 

On Oct 2, 2017, at 10:38 AM, Ben Newman <[hidden email]> wrote:

Taking a step back from the details of this proposal, I have some thoughts about why it seems to be struggling to find support.

In no particular order, I would say this proposal
  • relies on microbenchmarks, which can be misleading
  • disregards Amdahl's Law, by pretending that real-world JS CPU usage is commonly/ever dominated by min/max computations
  • replaces two O(n) loops with another O(n) loop that does slightly more work on each iteration, resulting in no complexity improvement, and a fairly modest (< 2x) constant factor improvement
  • doesn't seem to provide usability/learnability improvements for any particular group of JS developers (for example, novice programmers)
  • doesn't seem to prevent any common bugs in JS code
As a member of TC39, I regret that we have not provided a clearer set of criteria for what it takes to get a new function into the standard library. While I can't speak for the committee as a whole, my suspicion is that this proposal is unlikely to meet that standard. It's a fine idea, but so are many other functions that you can implement in a normal (non-standard) library.

I would also challenge the committee to think about (or link to!) any concrete written criteria that someone with an idea for a proposal could use to assess its chances of acceptance. Imagine how much time we could save!

Ben


_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Re: Math.minmax

J Decker
In reply to this post by Xavier Stouder


```js
const minMax = (arr=[]) => { if( arr.length==1 ) return [arr[0],arr[0]];
   const result=[Infinity, -Infinity]; 
   for( let i=0;i < arr.length;i++ ) { 
      result[0] = 
arr[i] < result[0] ? arr[i]: ((result[1] = arr[i] > result[1]? arr[i]: result[1]), result[0]) ;
   } 
   return result;
}

Would be slightly faster to only do the max if it's not a min. Which fails if there's only 1 number to compare.

On Mon, Oct 2, 2017 at 9:12 AM, Xavier Stouder <[hidden email]> wrote:
JDecker: Just added your solution on the benchmark, it beats every
others solution and it's a elegant solution.

Kai Zhu: We can't see the screenshot. But please take in consideration
that it's been a long time that ECMAScript isn't only used in webapp,
and that some of applications using it can eat more than a million
numbers.
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss


_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Re: Math.minmax

T.J. Crowder-2
In reply to this post by Ben Newman
On Mon, Oct 2, 2017 at 6:38 PM, Ben Newman <[hidden email]> wrote:
> I would also challenge the committee to think about (or link to!)
> any concrete written criteria that someone with an idea for a
> proposal could use to assess its chances of acceptance. Imagine
> how much time we could save!

That would be fantastic. When [I brought this up earlier this
year][1], the answer seemed to be that there were none and that there
was no consensus amongst the committee.

-- T.J. Crowder

[1]: https://esdiscuss.org/topic/guidelines-and-general-direction-for-standard-library-proposals
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Re: Math.minmax

T.J. Crowder-2
If you're reading on esdiscuss.org, that link got munged and I don't
seem able to edit my posts anymore. Here's a shortened version:
https://goo.gl/fioZmj

-- T.J.
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
Reply | Threaded
Open this post in threaded view
|

Re: Re: Math.minmax

Xavier Stouder
In reply to this post by Ben Newman
Thank you for your answer Ben!
I hope that TC39 will make the set of criteria clearer for a bright
futur.
_______________________________________________
es-discuss mailing list
[hidden email]
https://mail.mozilla.org/listinfo/es-discuss
12