Discussion:
[PATCH intel-driver 2/2] test: read jpeg test data from /dev/urandom
(too old to reply)
Scott D Phillips
2016-10-14 23:59:05 UTC
Permalink
Reading from urandom seems to be faster than using the standard
PRNG.

Signed-off-by: Scott D Phillips <***@intel.com>
---
test/i965_jpeg_test_data.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/i965_jpeg_test_data.cpp b/test/i965_jpeg_test_data.cpp
index 956f7cf..082d43f 100644
--- a/test/i965_jpeg_test_data.cpp
+++ b/test/i965_jpeg_test_data.cpp
@@ -28,6 +28,7 @@
#include "test_utils.h"

#include <algorithm>
+#include <fstream>
#include <numeric>

namespace JPEG {
@@ -988,9 +989,8 @@ namespace Encode {

TestInput::Shared input(TestInput::create(fourcc, res[0], res[1]));
if (input.get()) {
- std::generate_n(
- std::begin(input->bytes), input->bytes.size(),
- RandomValueGenerator<uint8_t>(0x00, 0xff));
+ std::basic_ifstream<uint8_t> urandom("/dev/urandom");
+ urandom.read(input->bytes.data(), input->bytes.size());
}
return input;
}
--
2.7.4
Scott D Phillips
2016-10-14 23:59:04 UTC
Permalink
std::valarray can fuse elementwise operations across arrays for
more efficient comparison.

Signed-off-by: Scott D Phillips <***@intel.com>
---
test/i965_jpeg_encode_test.cpp | 29 +++++++++++++----------------
1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/test/i965_jpeg_encode_test.cpp b/test/i965_jpeg_encode_test.cpp
index 29c14dc..209e80d 100644
--- a/test/i965_jpeg_encode_test.cpp
+++ b/test/i965_jpeg_encode_test.cpp
@@ -30,6 +30,7 @@
#include <cstring>
#include <memory>
#include <tuple>
+#include <valarray>

namespace JPEG {
namespace Encode {
@@ -400,23 +401,19 @@ protected:
ASSERT_EQ(expect->height(), image.height);
ASSERT_NO_FAILURE(uint8_t *data = mapBuffer<uint8_t>(image.buf));

- auto isClose = [](const uint8_t& a, const uint8_t& b) {
- return std::abs(int(a)-int(b)) <= 2;
- };
-
for (size_t i(0); i < image.num_planes; ++i) {
- size_t w = expect->widths[i];
- size_t h = expect->heights[i];
-
- const ByteData::value_type *source = expect->plane(i);
- const uint8_t *result = data + image.offsets[i];
- ASSERT_GE(image.pitches[i], w);
- for (size_t r(0); r < h; ++r) {
- EXPECT_TRUE(std::equal(result, result + w, source, isClose))
- << "Byte(s) mismatch in plane " << i << " row " << r;
- source += w;
- result += image.pitches[i];
- }
+ ASSERT_GE(image.pitches[i], expect->widths[i]);
+ std::valarray<int8_t> source(
+ reinterpret_cast<const int8_t *>(expect->plane(i)),
+ expect->widths[i] * expect->heights[i]);
+ std::gslice result_slice(0, {expect->heights[i], expect->widths[i]},
+ {image.pitches[i], 1});
+ std::valarray<int8_t> result = std::valarray<int8_t>(
+ reinterpret_cast<const int8_t *>(data + image.offsets[i]),
+ image.pitches[i] * expect->heights[i])[result_slice];
+ ASSERT_EQ(source.size(), result.size());
+ EXPECT_TRUE(std::abs(source - result).max() <= 2)
+ << "Byte(s) mismatch in plane " << i;
}

unmapBuffer(image.buf);
--
2.7.4
Eoff, Ullysses A
2016-10-15 02:13:06 UTC
Permalink
-----Original Message-----
From: Phillips, Scott D
Sent: Friday, October 14, 2016 4:59 PM
Subject: [PATCH intel-driver 1/2] test: use valarray for raw image comparison
std::valarray can fuse elementwise operations across arrays for
more efficient comparison.
---
test/i965_jpeg_encode_test.cpp | 29 +++++++++++++----------------
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/test/i965_jpeg_encode_test.cpp b/test/i965_jpeg_encode_test.cpp
index 29c14dc..209e80d 100644
--- a/test/i965_jpeg_encode_test.cpp
+++ b/test/i965_jpeg_encode_test.cpp
@@ -30,6 +30,7 @@
#include <cstring>
#include <memory>
#include <tuple>
+#include <valarray>
namespace JPEG {
namespace Encode {
ASSERT_EQ(expect->height(), image.height);
ASSERT_NO_FAILURE(uint8_t *data = mapBuffer<uint8_t>(image.buf));
- auto isClose = [](const uint8_t& a, const uint8_t& b) {
- return std::abs(int(a)-int(b)) <= 2;
- };
-
for (size_t i(0); i < image.num_planes; ++i) {
- size_t w = expect->widths[i];
- size_t h = expect->heights[i];
-
- const ByteData::value_type *source = expect->plane(i);
- const uint8_t *result = data + image.offsets[i];
- ASSERT_GE(image.pitches[i], w);
- for (size_t r(0); r < h; ++r) {
- EXPECT_TRUE(std::equal(result, result + w, source, isClose))
- << "Byte(s) mismatch in plane " << i << " row " << r;
- source += w;
- result += image.pitches[i];
- }
+ ASSERT_GE(image.pitches[i], expect->widths[i]);
+ std::valarray<int8_t> source(
+ reinterpret_cast<const int8_t *>(expect->plane(i)),
+ expect->widths[i] * expect->heights[i]);
expect->widths[i] * expect->heights[i] is already calculated in expect->sizes[i]
+ std::gslice result_slice(0, {expect->heights[i], expect->widths[i]},
+ {image.pitches[i], 1});
+ std::valarray<int8_t> result = std::valarray<int8_t>(
+ reinterpret_cast<const int8_t *>(data + image.offsets[i]),
+ image.pitches[i] * expect->heights[i])[result_slice];
+ ASSERT_EQ(source.size(), result.size());
+ EXPECT_TRUE(std::abs(source - result).max() <= 2)
+ << "Byte(s) mismatch in plane " << i;
}
unmapBuffer(image.buf);
--
2.7.4
Eoff, Ullysses A
2016-10-15 02:22:10 UTC
Permalink
Very nice speed improvement! I was not aware of std::valarray.

I have one minor comment in the first patch. Other than that, LGTM.

We don't have to do it now, but perhaps we could use a std::valarray as the
underlying type for ::JPEG::Encode::TestInput::bytes to avoid some of the extra
copies and reduce the memory usage footprint, too. But that would be an
optimization we can do in a later patchset.

Cheers,
U. Artie Eoff
-----Original Message-----
From: Phillips, Scott D
Sent: Friday, October 14, 2016 4:59 PM
Subject: [PATCH intel-driver 0/2] Speed up jpeg encode tests
For me these two patches take the Big encode test from 44sec to ~7sec.
test: use valarray for raw image comparison
test: read jpeg test data from /dev/urandom
test/i965_jpeg_encode_test.cpp | 29 +++++++++++++----------------
test/i965_jpeg_test_data.cpp | 6 +++---
2 files changed, 16 insertions(+), 19 deletions(-)
--
2.7.4
Xiang, Haihao
2016-10-17 04:20:49 UTC
Permalink
Hi Scott,

Does your patches change the pass criteria? I don't see any failure
after applying your patches, but Common/JPEGEncodeInputTest.Full/95
should be failed if no change to pass criteria.

Thanks
Haihao
For me these two patches take the Big encode test from 44sec to
~7sec.
  test: use valarray for raw image comparison
  test: read jpeg test data from /dev/urandom
 test/i965_jpeg_encode_test.cpp | 29 +++++++++++++----------------
 test/i965_jpeg_test_data.cpp   |  6 +++---
 2 files changed, 16 insertions(+), 19 deletions(-)
Scott D Phillips
2016-10-17 14:04:58 UTC
Permalink
Post by Xiang, Haihao
Hi Scott,
Does your patches change the pass criteria? I don't see any failure
after applying your patches, but Common/JPEGEncodeInputTest.Full/95
should be failed if no change to pass criteria.
It should not alter the pass-criteria for any test. Looking closer at the
change, because the comparison is done with int8_t, it is possible that a
surface of all 0 will compare as 'close enough' to a surface of all 255. Let me see if I
can fix that.
Post by Xiang, Haihao
Thanks
Haihao
For me these two patches take the Big encode test from 44sec to
~7sec.
  test: use valarray for raw image comparison
  test: read jpeg test data from /dev/urandom
 test/i965_jpeg_encode_test.cpp | 29 +++++++++++++----------------
 test/i965_jpeg_test_data.cpp   |  6 +++---
 2 files changed, 16 insertions(+), 19 deletions(-)
Sean V Kelley
2016-10-17 15:51:34 UTC
Permalink
On Mon, Oct 17, 2016 at 7:04 AM, Scott D Phillips
Post by Scott D Phillips
Post by Xiang, Haihao
Hi Scott,
Does your patches change the pass criteria? I don't see any failure
after applying your patches, but Common/JPEGEncodeInputTest.Full/95
should be failed if no change to pass criteria.
It should not alter the pass-criteria for any test. Looking closer at the
change, because the comparison is done with int8_t, it is possible that a
surface of all 0 will compare as 'close enough' to a surface of all 255. Let me see if I
can fix that.
Yes, I was seeing the same thing as Haihao, and was expecting the failure.

Sean
Post by Scott D Phillips
Post by Xiang, Haihao
Thanks
Haihao
For me these two patches take the Big encode test from 44sec to
~7sec.
test: use valarray for raw image comparison
test: read jpeg test data from /dev/urandom
test/i965_jpeg_encode_test.cpp | 29 +++++++++++++----------------
test/i965_jpeg_test_data.cpp | 6 +++---
2 files changed, 16 insertions(+), 19 deletions(-)
_______________________________________________
Libva mailing list
https://lists.freedesktop.org/mailman/listinfo/libva
--
Sean V. Kelley <***@intel.com>
Open Source Technology Center / SSG
Intel Corp.
Scott D Phillips
2016-10-17 20:00:51 UTC
Permalink
std::valarray can fuse elementwise operations across arrays for
more efficient comparison.

Signed-off-by: Scott D Phillips <***@intel.com>
---
Changes since v1:
- s/width * height/sizes/
- Added another array 'signs' which ensures that 0 and 255 never
compare as separated by one.

Also, the patch about /dev/urandom actually was accidentally
generating all zeroes instead of random data. Fixing the bug
there eliminated the performance gain, so I'm dropping that patch
from the series.

test/i965_jpeg_encode_test.cpp | 29 +++++++++++++----------------
1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/test/i965_jpeg_encode_test.cpp b/test/i965_jpeg_encode_test.cpp
index 29c14dc..173cd93 100644
--- a/test/i965_jpeg_encode_test.cpp
+++ b/test/i965_jpeg_encode_test.cpp
@@ -30,6 +30,7 @@
#include <cstring>
#include <memory>
#include <tuple>
+#include <valarray>

namespace JPEG {
namespace Encode {
@@ -400,23 +401,19 @@ protected:
ASSERT_EQ(expect->height(), image.height);
ASSERT_NO_FAILURE(uint8_t *data = mapBuffer<uint8_t>(image.buf));

- auto isClose = [](const uint8_t& a, const uint8_t& b) {
- return std::abs(int(a)-int(b)) <= 2;
- };
-
for (size_t i(0); i < image.num_planes; ++i) {
- size_t w = expect->widths[i];
- size_t h = expect->heights[i];
-
- const ByteData::value_type *source = expect->plane(i);
- const uint8_t *result = data + image.offsets[i];
- ASSERT_GE(image.pitches[i], w);
- for (size_t r(0); r < h; ++r) {
- EXPECT_TRUE(std::equal(result, result + w, source, isClose))
- << "Byte(s) mismatch in plane " << i << " row " << r;
- source += w;
- result += image.pitches[i];
- }
+ ASSERT_GE(image.pitches[i], expect->widths[i]);
+ std::valarray<uint8_t> source(expect->plane(i), expect->sizes[i]);
+ std::gslice result_slice(0, {expect->heights[i], expect->widths[i]},
+ {image.pitches[i], 1});
+ std::valarray<uint8_t> result = std::valarray<uint8_t>(
+ data + image.offsets[i],
+ image.pitches[i] * expect->heights[i])[result_slice];
+ std::valarray<uint8_t> signs(1, result.size());
+ signs[result > source] = -1;
+ ASSERT_EQ(source.size(), result.size());
+ EXPECT_TRUE((source * signs - result * signs).max() <= 2)
+ << "Byte(s) mismatch in plane " << i;
}

unmapBuffer(image.buf);
--
2.7.4
Eoff, Ullysses A
2016-10-18 18:38:02 UTC
Permalink
Ah, I overlooked the data types on my first review. Indeed it's important to take it into account when diff'ing. This is why they were being cast to "int" previously to calculate the diff.

On my system /dev/urandom seems to work (i.e. not all zeros). But I agree, let's drop it since it does not seem to be very uniform.

This version looks much better to me.

Thanks,

U. Artie
-----Original Message-----
Sent: Monday, October 17, 2016 1:01 PM
Subject: [Libva] [PATCH v2] test: use valarray for raw image comparison
std::valarray can fuse elementwise operations across arrays for
more efficient comparison.
---
- s/width * height/sizes/
- Added another array 'signs' which ensures that 0 and 255 never
compare as separated by one.
Also, the patch about /dev/urandom actually was accidentally
generating all zeroes instead of random data. Fixing the bug
there eliminated the performance gain, so I'm dropping that patch
from the series.
test/i965_jpeg_encode_test.cpp | 29 +++++++++++++----------------
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/test/i965_jpeg_encode_test.cpp b/test/i965_jpeg_encode_test.cpp
index 29c14dc..173cd93 100644
--- a/test/i965_jpeg_encode_test.cpp
+++ b/test/i965_jpeg_encode_test.cpp
@@ -30,6 +30,7 @@
#include <cstring>
#include <memory>
#include <tuple>
+#include <valarray>
namespace JPEG {
namespace Encode {
ASSERT_EQ(expect->height(), image.height);
ASSERT_NO_FAILURE(uint8_t *data = mapBuffer<uint8_t>(image.buf));
- auto isClose = [](const uint8_t& a, const uint8_t& b) {
- return std::abs(int(a)-int(b)) <= 2;
- };
-
for (size_t i(0); i < image.num_planes; ++i) {
- size_t w = expect->widths[i];
- size_t h = expect->heights[i];
-
- const ByteData::value_type *source = expect->plane(i);
- const uint8_t *result = data + image.offsets[i];
- ASSERT_GE(image.pitches[i], w);
- for (size_t r(0); r < h; ++r) {
- EXPECT_TRUE(std::equal(result, result + w, source, isClose))
- << "Byte(s) mismatch in plane " << i << " row " << r;
- source += w;
- result += image.pitches[i];
- }
+ ASSERT_GE(image.pitches[i], expect->widths[i]);
+ std::valarray<uint8_t> source(expect->plane(i), expect->sizes[i]);
+ std::gslice result_slice(0, {expect->heights[i], expect->widths[i]},
+ {image.pitches[i], 1});
+ std::valarray<uint8_t> result = std::valarray<uint8_t>(
+ data + image.offsets[i],
+ image.pitches[i] * expect->heights[i])[result_slice];
+ std::valarray<uint8_t> signs(1, result.size());
+ signs[result > source] = -1;
+ ASSERT_EQ(source.size(), result.size());
+ EXPECT_TRUE((source * signs - result * signs).max() <= 2)
+ << "Byte(s) mismatch in plane " << i;
}
unmapBuffer(image.buf);
--
2.7.4
_______________________________________________
Libva mailing list
https://lists.freedesktop.org/mailman/listinfo/libva
Xiang, Haihao
2016-10-19 13:34:31 UTC
Permalink
It works well for me, applied.

Thanks
Haihao
-----Original Message-----
Ullysses A
Sent: Wednesday, October 19, 2016 2:38 AM
Subject: Re: [Libva] [PATCH v2] test: use valarray for raw image comparison
Ah, I overlooked the data types on my first review. Indeed it's important to
take it into account when diff'ing. This is why they were being cast to "int"
previously to calculate the diff.
On my system /dev/urandom seems to work (i.e. not all zeros). But I agree,
let's drop it since it does not seem to be very uniform.
This version looks much better to me.
Thanks,
U. Artie
-----Original Message-----
Sent: Monday, October 17, 2016 1:01 PM
Subject: [Libva] [PATCH v2] test: use valarray for raw image
comparison
std::valarray can fuse elementwise operations across arrays for more
efficient comparison.
---
- s/width * height/sizes/
- Added another array 'signs' which ensures that 0 and 255 never
compare as separated by one.
Also, the patch about /dev/urandom actually was accidentally
generating all zeroes instead of random data. Fixing the bug there
eliminated the performance gain, so I'm dropping that patch from the
series.
test/i965_jpeg_encode_test.cpp | 29 +++++++++++++----------------
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/test/i965_jpeg_encode_test.cpp
b/test/i965_jpeg_encode_test.cpp index 29c14dc..173cd93 100644
--- a/test/i965_jpeg_encode_test.cpp
+++ b/test/i965_jpeg_encode_test.cpp
@@ -30,6 +30,7 @@
#include <cstring>
#include <memory>
#include <tuple>
+#include <valarray>
namespace JPEG {
namespace Encode {
ASSERT_EQ(expect->height(), image.height);
ASSERT_NO_FAILURE(uint8_t *data =
mapBuffer<uint8_t>(image.buf));
- auto isClose = [](const uint8_t& a, const uint8_t& b) {
- return std::abs(int(a)-int(b)) <= 2;
- };
-
for (size_t i(0); i < image.num_planes; ++i) {
- size_t w = expect->widths[i];
- size_t h = expect->heights[i];
-
- const ByteData::value_type *source = expect->plane(i);
- const uint8_t *result = data + image.offsets[i];
- ASSERT_GE(image.pitches[i], w);
- for (size_t r(0); r < h; ++r) {
- EXPECT_TRUE(std::equal(result, result + w, source, isClose))
- << "Byte(s) mismatch in plane " << i << " row " << r;
- source += w;
- result += image.pitches[i];
- }
+ ASSERT_GE(image.pitches[i], expect->widths[i]);
+ std::valarray<uint8_t> source(expect->plane(i), expect->sizes[i]);
+ std::gslice result_slice(0, {expect->heights[i], expect->widths[i]},
+ {image.pitches[i], 1});
+ std::valarray<uint8_t> result = std::valarray<uint8_t>(
+ data + image.offsets[i],
+ image.pitches[i] * expect->heights[i])[result_slice];
+ std::valarray<uint8_t> signs(1, result.size());
+ signs[result > source] = -1;
+ ASSERT_EQ(source.size(), result.size());
+ EXPECT_TRUE((source * signs - result * signs).max() <= 2)
+ << "Byte(s) mismatch in plane " << i;
}
unmapBuffer(image.buf);
--
2.7.4
_______________________________________________
Libva mailing list
https://lists.freedesktop.org/mailman/listinfo/libva
_______________________________________________
Libva mailing list
https://lists.freedesktop.org/mailman/listinfo/libva
Loading...