Use simple/fast macro version of MinMax in JS

Use simple/fast macro version of MinMax in JS

Use the simple macro version of {Min, Max} where possible to
improve performance

Follow-up to CR: https://codereview.chromium.org/1331993004

BUG=

Committed: https://crrev.com/27c96c26212a10bb7f19f7bf3ff793b31bbad354
Cr-Commit-Position: refs/heads/master@{#31162}

[skomski, 2015-10-07 08:46:58]

karl@skomski.com changed reviewers:
+ jkummerow@chromium.org, littledan@chromium.org

[skomski, 2015-10-07 08:47:31]

CR version without the cleanup in https://codereview.chromium.org/1345333002/

[Dan Ehrenberg, 2015-10-07 17:10:23]

lgtm

[skomski, 2015-10-07 18:06:53]

The CQ bit was checked by karl@skomski.com to run a CQ dry run

[commit-bot: I haz the power, 2015-10-07 18:07:02]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1390023003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1390023003/20001

[commit-bot: I haz the power, 2015-10-07 18:30:37]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-10-07 18:30:37]

Dry run: This issue passed the CQ dry run.

[skomski, 2015-10-07 18:31:17]

The CQ bit was checked by karl@skomski.com

[commit-bot: I haz the power, 2015-10-07 18:31:38]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1390023003/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1390023003/20001

[commit-bot: I haz the power, 2015-10-07 18:32:52]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2015-10-07 18:33:14]

Patchset 2 (id:??) landed as
https://crrev.com/27c96c26212a10bb7f19f7bf3ff793b31bbad354
Cr-Commit-Position: refs/heads/master@{#31162}

[Michael Starzinger, 2015-10-07 19:02:22]

mstarzinger@chromium.org changed reviewers:
+ mstarzinger@chromium.org

[Michael Starzinger, 2015-10-07 19:02:23]

Aehm ...

https://codereview.chromium.org/1390023003/diff/20001/src/harmony-atomics.js
File src/harmony-atomics.js (right):

https://codereview.chromium.org/1390023003/diff/20001/src/harmony-atomics.js#...
src/harmony-atomics.js:156: count = MAX_SIMPLE(0, TO_INTEGER(count));
I don't have a machine where I could verify this right now, but I am pretty sure
this is observable now ...

Atomics.futexWait(a, 0, something_that_observes_ToNumber)

https://codereview.chromium.org/1390023003/diff/20001/src/harmony-atomics.js#...
src/harmony-atomics.js:163: count = MAX_SIMPLE(0, TO_INTEGER(count));
Likewise.

[skomski, 2015-10-08 10:37:34]

On 2015/10/07 19:02:23, Michael Starzinger wrote:
> Aehm ...
> 
> https://codereview.chromium.org/1390023003/diff/20001/src/harmony-atomics.js
> File src/harmony-atomics.js (right):
> 
>
https://codereview.chromium.org/1390023003/diff/20001/src/harmony-atomics.js#...
> src/harmony-atomics.js:156: count = MAX_SIMPLE(0, TO_INTEGER(count));
> I don't have a machine where I could verify this right now, but I am pretty
sure
> this is observable now ...
> 
> Atomics.futexWait(a, 0, something_that_observes_ToNumber)
> 
>
https://codereview.chromium.org/1390023003/diff/20001/src/harmony-atomics.js#...
> src/harmony-atomics.js:163: count = MAX_SIMPLE(0, TO_INTEGER(count));
> Likewise.

To answer: https://codereview.chromium.org/1331993004/#msg17

Thanks for reviewing!
Yes I was aware that it would evaluate twice but it was fine for subarray
because performance-wise there
was no notable difference versus my only evaluated once if statement version and
no runtime function was evaluated twice.

The problem with using the %IS_VAR marker is first it doesn't work for constants
and second the readability/simplicity of
the macro would be lost why it was introduced in the first place if a new var is
needed for every expression.
But of course it should guard against misuse but what is the best way. The best
thing of course would be temp variables in the macro
but I don't think this is possible.

For now I will make new CR to fix the misuse in src/harmony-atomics.js and
json.js with added tests or should I revert this?

[Jakob Kummerow, 2015-10-08 11:32:07]

The macro absolutely needs to be hardened against accidental misuse, either by
beefing it up (if that's possible) or by adding an %IS_VAR check and fixing all
the call sites. If you think the resulting verbosity defeats the point of having
the macro, then feel free to remove it again and go back to MathMin/MathMax
(revert this CL and the other one).

Easily-misused, unsafe micro-optimizations are definitely not the way to go. We
have enough of those as it is, and are slowly fighting our way towards
eliminating them.

[Dan Ehrenberg, 2015-10-08 20:37:27]

A revert of this CL (patchset #2 id:20001) has been created in
https://codereview.chromium.org/1394303003/ by littledan@chromium.org.

The reason for reverting is: This contains bugs, as found by mstarzinger.
Reverting until we can find a clean fix (maybe it should be an inline function
instead of a macro)..