[PATCH] Implement std::experimental::sample

Eric Fiselier eric at efcs.ca
Thu Apr 30 10:01:45 PDT 2015


In http://reviews.llvm.org/D9044#162287, @eugenis wrote:

> > However because sample is a function it can be looked up by ADL. Since sample takes user-defined iterator types and RNG's it is possible that existing unqualified calls to sample in user code may now find std::sample.
>
>
> No, wait, there is no std::sample. It's std::experimental::sample. And the bad ADL could only happen if one of the arguments is in std::experimental, which probably means it's intentional.
>
> ../1.cc:9:3: error: no member named 'sample' in namespace 'std'; did you mean 'std::experimental::sample'?
>
>   std::sample(a, b, c, d, t);


Urg, That was a stupid mistake on my part. Sorry and thanks for the correction.


================
Comment at: include/experimental/algorithm:40
@@ +39,3 @@
+#include <algorithm>
+
+_LIBCPP_BEGIN_NAMESPACE_LFTS
----------------
This should probably have the system header pragma found in most other headers.

================
Comment at: include/experimental/algorithm:53
@@ +52,3 @@
+  for (; __first != __last; ++__first, (void)++__k) {
+    _Distance __r = _VSTD::uniform_int_distribution<>(0, __k)(__g);
+    if (__r < __sz)
----------------
eugenis wrote:
> This thing is costing us lots of performance - it's just way too fancy.
> It makes this code is about 2x slower than std::random_sample in existing implementations, but I guess it makes the sample more uniform, as well.
> 
> Do you thing smth like (__g() - __g::min()) % ( __k + 1) would be acceptable?
Not sure, but let's do it correctly first then apply the optimization.

http://reviews.llvm.org/D9044

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list