You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Ross Allen <ro...@mesosphere.io> on 2013/11/15 19:13:54 UTC

Review Request 15579: Saved KB/MB/GB bytes as constants to save multiplication.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15579/
-----------------------------------------------------------

Review request for mesos.


Repository: mesos-git


Description
-------

Saved KB/MB/GB bytes as constants to save multiplication.

Each time the 'dataSize' filter was called, the function did lots of
multiplication to get bytes per KB, MB, and GB to determine how to
print the string.

Moving the constants outside the function returned by the filter means
the multiplication is done only one time on app start up instead of in
each call.

* Added 1 digit to the right of the decimal point for GB. 6.5 GB is
  very different from 7 GB.


Diffs
-----

  src/webui/master/static/js/app.js 163c50b6cb88f33a23a1a3446d1a88070be26eab 

Diff: https://reviews.apache.org/r/15579/diff/


Testing
-------


Thanks,

Ross Allen


Re: Review Request 15579: Saved KB/MB/GB bytes as constants to save multiplication.

Posted by Ben Mahler <be...@gmail.com>.

> On Nov. 15, 2013, 10:33 p.m., Ben Mahler wrote:
> > src/webui/master/static/js/app.js, lines 86-88
> > <https://reviews.apache.org/r/15579/diff/1/?file=385447#file385447line86>
> >
> >     Good call, here's how we set up the variables in bytes.hpp, seems very obviously correct, maybe we could to the same here?
> >     
> >       static const uint64_t BYTES = 1;
> >       static const uint64_t KILOBYTES = 1024 * BYTES;
> >       static const uint64_t MEGABYTES = 1024 * KILOBYTES;
> >       static const uint64_t GIGABYTES = 1024 * MEGABYTES;
> >       static const uint64_t TERABYTES = 1024 * GIGABYTES;
> 
> Ross Allen wrote:
>     But where does 1024 come from? It's 2^10!
>     
>     That should be:
>     
>     BYTES = 2^0
>     KILOBYTES = 2^10
>     MEGABYTES = 2^20
>     GIGABYTES = 2^30
>     TERABYTES = 2^40
>     
>     I was tempted to go with the newer kibibyte, mebibyte, and gibibyte, but even I'm not sold on the names yet. They're more proper, but they sound odd.

While they are the same in value, my point here was twofold:

1. We should be consistent within our code (let's not do the same thing different ways).
2. TERABYTES = 1024 * GIGABYTES is more obviously correct to someone reading the code than TERABYTES = Math.pow(2,40). But this may be subjective if there are people who think naturally in terms of exponentiation, I certainly don't ;)


- Ben


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15579/#review28995
-----------------------------------------------------------


On Nov. 15, 2013, 6:13 p.m., Ross Allen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15579/
> -----------------------------------------------------------
> 
> (Updated Nov. 15, 2013, 6:13 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Saved KB/MB/GB bytes as constants to save multiplication.
> 
> Each time the 'dataSize' filter was called, the function did lots of
> multiplication to get bytes per KB, MB, and GB to determine how to
> print the string.
> 
> Moving the constants outside the function returned by the filter means
> the multiplication is done only one time on app start up instead of in
> each call.
> 
> * Added 1 digit to the right of the decimal point for GB. 6.5 GB is
>   very different from 7 GB.
> 
> 
> Diffs
> -----
> 
>   src/webui/master/static/js/app.js 163c50b6cb88f33a23a1a3446d1a88070be26eab 
> 
> Diff: https://reviews.apache.org/r/15579/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ross Allen
> 
>


Re: Review Request 15579: Saved KB/MB/GB bytes as constants to save multiplication.

Posted by Ross Allen <ro...@mesosphere.io>.

> On Nov. 15, 2013, 10:33 p.m., Ben Mahler wrote:
> > src/webui/master/static/js/app.js, lines 86-88
> > <https://reviews.apache.org/r/15579/diff/1/?file=385447#file385447line86>
> >
> >     Good call, here's how we set up the variables in bytes.hpp, seems very obviously correct, maybe we could to the same here?
> >     
> >       static const uint64_t BYTES = 1;
> >       static const uint64_t KILOBYTES = 1024 * BYTES;
> >       static const uint64_t MEGABYTES = 1024 * KILOBYTES;
> >       static const uint64_t GIGABYTES = 1024 * MEGABYTES;
> >       static const uint64_t TERABYTES = 1024 * GIGABYTES;

But where does 1024 come from? It's 2^10!

That should be:

BYTES = 2^0
KILOBYTES = 2^10
MEGABYTES = 2^20
GIGABYTES = 2^30
TERABYTES = 2^40

I was tempted to go with the newer kibibyte, mebibyte, and gibibyte, but even I'm not sold on the names yet. They're more proper, but they sound odd.


- Ross


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15579/#review28995
-----------------------------------------------------------


On Nov. 15, 2013, 6:13 p.m., Ross Allen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15579/
> -----------------------------------------------------------
> 
> (Updated Nov. 15, 2013, 6:13 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Saved KB/MB/GB bytes as constants to save multiplication.
> 
> Each time the 'dataSize' filter was called, the function did lots of
> multiplication to get bytes per KB, MB, and GB to determine how to
> print the string.
> 
> Moving the constants outside the function returned by the filter means
> the multiplication is done only one time on app start up instead of in
> each call.
> 
> * Added 1 digit to the right of the decimal point for GB. 6.5 GB is
>   very different from 7 GB.
> 
> 
> Diffs
> -----
> 
>   src/webui/master/static/js/app.js 163c50b6cb88f33a23a1a3446d1a88070be26eab 
> 
> Diff: https://reviews.apache.org/r/15579/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ross Allen
> 
>


Re: Review Request 15579: Saved KB/MB/GB bytes as constants to save multiplication.

Posted by Ben Mahler <be...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/15579/#review28995
-----------------------------------------------------------

Ship it!



src/webui/master/static/js/app.js
<https://reviews.apache.org/r/15579/#comment56042>

    Good call, here's how we set up the variables in bytes.hpp, seems very obviously correct, maybe we could to the same here?
    
      static const uint64_t BYTES = 1;
      static const uint64_t KILOBYTES = 1024 * BYTES;
      static const uint64_t MEGABYTES = 1024 * KILOBYTES;
      static const uint64_t GIGABYTES = 1024 * MEGABYTES;
      static const uint64_t TERABYTES = 1024 * GIGABYTES;


- Ben Mahler


On Nov. 15, 2013, 6:13 p.m., Ross Allen wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/15579/
> -----------------------------------------------------------
> 
> (Updated Nov. 15, 2013, 6:13 p.m.)
> 
> 
> Review request for mesos.
> 
> 
> Repository: mesos-git
> 
> 
> Description
> -------
> 
> Saved KB/MB/GB bytes as constants to save multiplication.
> 
> Each time the 'dataSize' filter was called, the function did lots of
> multiplication to get bytes per KB, MB, and GB to determine how to
> print the string.
> 
> Moving the constants outside the function returned by the filter means
> the multiplication is done only one time on app start up instead of in
> each call.
> 
> * Added 1 digit to the right of the decimal point for GB. 6.5 GB is
>   very different from 7 GB.
> 
> 
> Diffs
> -----
> 
>   src/webui/master/static/js/app.js 163c50b6cb88f33a23a1a3446d1a88070be26eab 
> 
> Diff: https://reviews.apache.org/r/15579/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ross Allen
> 
>