You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Joris Van Remoortere <jo...@gmail.com> on 2015/07/27 09:59:54 UTC
Review Request 36845: Stout: Introduced TLS wrapper for thread local
storage.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36845/
-----------------------------------------------------------
Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Michael Park.
Bugs: MESOS-3119
https://issues.apache.org/jira/browse/MESOS-3119
Repository: mesos
Description
-------
This replaced the ThreadLocal primitive with the new c++11 standard.
The exception is on OSX where we use `__thread` as thread_local is not
supported.
Diffs
-----
3rdparty/libprocess/3rdparty/stout/Makefile.am 89e7b1854bd7f449f4f0027d76c6430d259a24de
3rdparty/libprocess/3rdparty/stout/include/Makefile.am 5c19e3ef8ba50ab007eda26b752441f076ca7ed0
3rdparty/libprocess/3rdparty/stout/include/stout/thread.hpp 552d6e97c882a36d6a889af205c422e51f544b34
3rdparty/libprocess/3rdparty/stout/include/stout/thread_local.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/tests/thread_tests.cpp 319fcdf517b24f5bb9c85dad4093b09ec87e915e
Diff: https://reviews.apache.org/r/36845/diff/
Testing
-------
dependens on follow-up patch.
Thanks,
Joris Van Remoortere
Re: Review Request 36845: Stout: Introduced THREAD_LOCAL wrapper for
thread local storage.
Posted by Artem Harutyunyan <ar...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36845/#review93298
-----------------------------------------------------------
Ship it!
3rdparty/libprocess/3rdparty/stout/include/stout/thread_local.hpp (line 25)
<https://reviews.apache.org/r/36845/#comment147675>
Also, by default TLS for a lot of people is Transport Layer Security. So this name will likely cause some brain cycle waste every now and then.
- Artem Harutyunyan
On July 27, 2015, 2:05 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36845/
> -----------------------------------------------------------
>
> (Updated July 27, 2015, 2:05 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Michael Park.
>
>
> Bugs: MESOS-3119
> https://issues.apache.org/jira/browse/MESOS-3119
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This replaced the ThreadLocal primitive with the new c++11 standard.
> The exception is on OSX where we use `__thread` as thread_local is not
> supported.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/Makefile.am 89e7b1854bd7f449f4f0027d76c6430d259a24de
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 5c19e3ef8ba50ab007eda26b752441f076ca7ed0
> 3rdparty/libprocess/3rdparty/stout/include/stout/thread.hpp 552d6e97c882a36d6a889af205c422e51f544b34
> 3rdparty/libprocess/3rdparty/stout/include/stout/thread_local.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/tests/thread_tests.cpp 319fcdf517b24f5bb9c85dad4093b09ec87e915e
>
> Diff: https://reviews.apache.org/r/36845/diff/
>
>
> Testing
> -------
>
> dependens on follow-up patch.
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 36845: Stout: Introduced THREAD_LOCAL wrapper for
thread local storage.
Posted by Artem Harutyunyan <ar...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36845/#review93230
-----------------------------------------------------------
Ship it!
3rdparty/libprocess/3rdparty/stout/include/stout/thread_local.hpp (line 20)
<https://reviews.apache.org/r/36845/#comment147553>
micro-nit #1: Is there a link explaining this that is not behind the registration wall?
3rdparty/libprocess/3rdparty/stout/include/stout/thread_local.hpp (line 22)
<https://reviews.apache.org/r/36845/#comment147554>
micro nit #2: either s/required/require/, or point to where the requirement was made.
- Artem Harutyunyan
On July 27, 2015, 2:05 p.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36845/
> -----------------------------------------------------------
>
> (Updated July 27, 2015, 2:05 p.m.)
>
>
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Michael Park.
>
>
> Bugs: MESOS-3119
> https://issues.apache.org/jira/browse/MESOS-3119
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This replaced the ThreadLocal primitive with the new c++11 standard.
> The exception is on OSX where we use `__thread` as thread_local is not
> supported.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/Makefile.am 89e7b1854bd7f449f4f0027d76c6430d259a24de
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 5c19e3ef8ba50ab007eda26b752441f076ca7ed0
> 3rdparty/libprocess/3rdparty/stout/include/stout/thread.hpp 552d6e97c882a36d6a889af205c422e51f544b34
> 3rdparty/libprocess/3rdparty/stout/include/stout/thread_local.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/tests/thread_tests.cpp 319fcdf517b24f5bb9c85dad4093b09ec87e915e
>
> Diff: https://reviews.apache.org/r/36845/diff/
>
>
> Testing
> -------
>
> dependens on follow-up patch.
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 36845: Stout: Introduced THREAD_LOCAL wrapper for
thread local storage.
Posted by Joris Van Remoortere <jo...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36845/
-----------------------------------------------------------
(Updated July 27, 2015, 9:05 p.m.)
Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Michael Park.
Changes
-------
renamed TLS to THREAD_LOCAL
Summary (updated)
-----------------
Stout: Introduced THREAD_LOCAL wrapper for thread local storage.
Bugs: MESOS-3119
https://issues.apache.org/jira/browse/MESOS-3119
Repository: mesos
Description
-------
This replaced the ThreadLocal primitive with the new c++11 standard.
The exception is on OSX where we use `__thread` as thread_local is not
supported.
Diffs (updated)
-----
3rdparty/libprocess/3rdparty/stout/Makefile.am 89e7b1854bd7f449f4f0027d76c6430d259a24de
3rdparty/libprocess/3rdparty/stout/include/Makefile.am 5c19e3ef8ba50ab007eda26b752441f076ca7ed0
3rdparty/libprocess/3rdparty/stout/include/stout/thread.hpp 552d6e97c882a36d6a889af205c422e51f544b34
3rdparty/libprocess/3rdparty/stout/include/stout/thread_local.hpp PRE-CREATION
3rdparty/libprocess/3rdparty/stout/tests/thread_tests.cpp 319fcdf517b24f5bb9c85dad4093b09ec87e915e
Diff: https://reviews.apache.org/r/36845/diff/
Testing
-------
dependens on follow-up patch.
Thanks,
Joris Van Remoortere
Re: Review Request 36845: Stout: Introduced TLS wrapper for thread
local storage.
Posted by Benjamin Hindman <be...@berkeley.edu>.
> On July 27, 2015, 5:54 p.m., Joseph Wu wrote:
> > 3rdparty/libprocess/3rdparty/stout/include/stout/thread_local.hpp, line 25
> > <https://reviews.apache.org/r/36845/diff/1/?file=1022249#file1022249line25>
> >
> > `TLS` might be a bit too short for readable code. (And people that google it will come up with Transport Layer Security, instead of Thread Local Storage.)
> > Maybe `THREAD_LOCAL` would be better.
I like THREAD_LOCAL too, it approximates 'thread_local' pretty well. ;-)
- Benjamin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36845/#review93142
-----------------------------------------------------------
On July 27, 2015, 7:59 a.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36845/
> -----------------------------------------------------------
>
> (Updated July 27, 2015, 7:59 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Michael Park.
>
>
> Bugs: MESOS-3119
> https://issues.apache.org/jira/browse/MESOS-3119
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This replaced the ThreadLocal primitive with the new c++11 standard.
> The exception is on OSX where we use `__thread` as thread_local is not
> supported.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/Makefile.am 89e7b1854bd7f449f4f0027d76c6430d259a24de
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 5c19e3ef8ba50ab007eda26b752441f076ca7ed0
> 3rdparty/libprocess/3rdparty/stout/include/stout/thread.hpp 552d6e97c882a36d6a889af205c422e51f544b34
> 3rdparty/libprocess/3rdparty/stout/include/stout/thread_local.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/tests/thread_tests.cpp 319fcdf517b24f5bb9c85dad4093b09ec87e915e
>
> Diff: https://reviews.apache.org/r/36845/diff/
>
>
> Testing
> -------
>
> dependens on follow-up patch.
>
>
> Thanks,
>
> Joris Van Remoortere
>
>
Re: Review Request 36845: Stout: Introduced TLS wrapper for thread
local storage.
Posted by Joseph Wu <jo...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36845/#review93142
-----------------------------------------------------------
Ship it!
3rdparty/libprocess/3rdparty/stout/include/stout/thread_local.hpp (line 25)
<https://reviews.apache.org/r/36845/#comment147404>
`TLS` might be a bit too short for readable code. (And people that google it will come up with Transport Layer Security, instead of Thread Local Storage.)
Maybe `THREAD_LOCAL` would be better.
- Joseph Wu
On July 27, 2015, 12:59 a.m., Joris Van Remoortere wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36845/
> -----------------------------------------------------------
>
> (Updated July 27, 2015, 12:59 a.m.)
>
>
> Review request for mesos, Benjamin Hindman, Artem Harutyunyan, and Michael Park.
>
>
> Bugs: MESOS-3119
> https://issues.apache.org/jira/browse/MESOS-3119
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This replaced the ThreadLocal primitive with the new c++11 standard.
> The exception is on OSX where we use `__thread` as thread_local is not
> supported.
>
>
> Diffs
> -----
>
> 3rdparty/libprocess/3rdparty/stout/Makefile.am 89e7b1854bd7f449f4f0027d76c6430d259a24de
> 3rdparty/libprocess/3rdparty/stout/include/Makefile.am 5c19e3ef8ba50ab007eda26b752441f076ca7ed0
> 3rdparty/libprocess/3rdparty/stout/include/stout/thread.hpp 552d6e97c882a36d6a889af205c422e51f544b34
> 3rdparty/libprocess/3rdparty/stout/include/stout/thread_local.hpp PRE-CREATION
> 3rdparty/libprocess/3rdparty/stout/tests/thread_tests.cpp 319fcdf517b24f5bb9c85dad4093b09ec87e915e
>
> Diff: https://reviews.apache.org/r/36845/diff/
>
>
> Testing
> -------
>
> dependens on follow-up patch.
>
>
> Thanks,
>
> Joris Van Remoortere
>
>