mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Nikita Vetoshkin" <nikita.vetosh...@gmail.com>
Subject Re: Review Request 34703: Added stream manipulators for the Time object.
Date Fri, 29 May 2015 13:30:47 GMT


> On May 28, 2015, 5:49 a.m., Nikita Vetoshkin wrote:
> > 3rdparty/libprocess/include/process/time.hpp, line 109
> > <https://reviews.apache.org/r/34703/diff/2/?file=972867#file972867line109>
> >
> >     Can I add one more nitpick? If we make `WEEK_DAYS` and `MONTHS` `static` then
will put only array (which is initialised staticly and save in binary) pointer on stack instead
of recreating array every time.
> >     Here's my code:
> >     
> >     ```
> >     const char *WEEK_DAYS[] = {"Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"};
> >     std::cout << WEEK_DAYS[i] << std::endl;
> >     ```
> >     Here's a relevant snippet of g++ assempler output:
> >     
> >     ```
> >             movq    $.LC0, (%rsp)
> >             movq    $.LC1, 8(%rsp)
> >             movq    $.LC2, 16(%rsp)
> >             movq    $.LC3, 24(%rsp)
> >             movq    $.LC4, 32(%rsp)
> >             movq    $.LC5, 40(%rsp)
> >             movq    $.LC6, 48(%rsp)
> >             movq    (%rsp,%rdi,8), %rsi
> >             movl    $_ZSt4cout, %edi
> >     ```
> >     If we make it `const`, then it starts looking like this:
> >     
> >     ```
> >             movq    _ZZ4mainE9WEEK_DAYS(,%rdi,8), %rsi
> >             movl    $_ZSt4cout, %edi
> >     ```
> 
> Alexander Rojas wrote:
>     The project itself is not so fond of using static function objects (I tried). The
idea is to leave it as const and later on marked as constexpr.
> 
> Alexander Rojas wrote:
>     I added constexpr. I haven't check the assembler output, but ir should make the trick.
However, if it doesn't, I'm dropping this issue since static function variables are not used
within mesos.

No, it does not. Array is constructed each time :)


- Nikita


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


On May 29, 2015, 12:59 p.m., Alexander Rojas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34703/
> -----------------------------------------------------------
> 
> (Updated May 29, 2015, 12:59 p.m.)
> 
> 
> Review request for mesos, Bernd Mathiske, Michael Park, Nikita Vetoshkin, and Till Toenshoff.
> 
> 
> Bugs: mesos-708
>     https://issues.apache.org/jira/browse/mesos-708
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Adds some manipulator classes which allows formatting Time objects to ostreams.
> 
> 
> Diffs
> -----
> 
>   3rdparty/libprocess/include/process/time.hpp c5ab2a3cfa83590eb6612152ae365dd67f51cea9

>   3rdparty/libprocess/src/tests/time_tests.cpp be314182c65c05d439b81aa5248a71d93f6f0a0b

> 
> Diff: https://reviews.apache.org/r/34703/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Alexander Rojas
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message