[PATCH v1 0/5] harden bytes_mutable_view and bytes_view

[Benny Halevy]

Currently bytes_mutable_view is defined in scylla
while bytes_view uses std::basic_string_view<int8_t>.

Both lack cheks for access out of range and may facilitate
undefined behavior.

This series hardens bytes_mutable_view and bytes_vies
by adding asserts checking access within the view boundaries
and marking most access functions noexcept. A common
template - `basic_view_base` - is used by both
to share the low-level logic.

A possibly-throwing `at()` method was added.
It checks the provided index and throws std::out_of_range
if needed.

perf_simple_query:

Before:
median 182633.31 tps ( 87.1 allocs/op, 20.1 tasks/op, 53727 insns/op)
median absolute deviation: 1094.85
maximum: 183728.16
minimum: 164361.01

After:
median 187516.88 tps ( 87.1 allocs/op, 20.1 tasks/op, 54160 insns/op)
median absolute deviation: 572.40
maximum: 188089.28
minimum: 186214.28

Test: unit(dev)

--
Available also on g...@github.com:bhalevy/scylla.git bytes_view_hardening-v1

Benny Halevy (5):
basic_mutable_view: mark trivial functions noexcept
basic_mutable_view: assert boundaries and mark functions noexcept
basic_mutable_view: add at() method
mutable_view: define and use basic_view_base for basic_mutable_view
utils: mutable_view: use basic_view_base for both basic_view and
basic_mutable_view

bytes.cc | 8 +-
bytes.hh | 18 +++-
test/boost/managed_bytes_test.cc | 8 ++
utils/fragment_range.hh | 3 +-
utils/mutable_view.hh | 168 ++++++++++++++++++++++++++-----
5 files changed, 170 insertions(+), 35 deletions(-)

--
2.31.1

[Benny Halevy]

Signed-off-by: Benny Halevy <bha...@scylladb.com>
---
test/boost/managed_bytes_test.cc | 8 ++++++++
utils/mutable_view.hh | 22 +++++++++++-----------
2 files changed, 19 insertions(+), 11 deletions(-)

diff --git a/test/boost/managed_bytes_test.cc b/test/boost/managed_bytes_test.cc
index 987d6c757..edff9f2ec 100644
--- a/test/boost/managed_bytes_test.cc
+++ b/test/boost/managed_bytes_test.cc
@@ -27,6 +27,14 @@
#include <boost/test/unit_test.hpp>
#include <unordered_set>

+static_assert(std::is_nothrow_constructible_v<bytes_view>);
+static_assert(std::is_nothrow_copy_constructible_v<bytes_view>);
+static_assert(std::is_nothrow_move_constructible_v<bytes_view>);
+
+static_assert(std::is_nothrow_constructible_v<bytes_mutable_view>);
+static_assert(std::is_nothrow_copy_constructible_v<bytes_mutable_view>);
+static_assert(std::is_nothrow_move_constructible_v<bytes_mutable_view>);
+
struct fragmenting_allocation_strategy : standard_allocation_strategy {
size_t allocated_bytes = 0;
std::unordered_set<void*> allocations;
diff --git a/utils/mutable_view.hh b/utils/mutable_view.hh
index 076135c32..1f5078213 100644
--- a/utils/mutable_view.hh
+++ b/utils/mutable_view.hh
@@ -39,12 +39,12 @@ class basic_mutable_view {
basic_mutable_view() = default;

template<typename U, U N>
- basic_mutable_view(basic_sstring<CharT, U, N>& str)
+ basic_mutable_view(basic_sstring<CharT, U, N>& str) noexcept
: _begin(str.begin())
, _end(str.end())
{ }

- basic_mutable_view(CharT* ptr, size_t length)
+ basic_mutable_view(CharT* ptr, size_t length) noexcept
: _begin(ptr)
, _end(ptr + length)
{ }
@@ -55,19 +55,19 @@ class basic_mutable_view {

CharT& operator[](size_t idx) const { return _begin[idx]; }

- iterator begin() const { return _begin; }
- iterator end() const { return _end; }
+ iterator begin() const noexcept { return _begin; }
+ iterator end() const noexcept { return _end; }

- CharT* data() const { return _begin; }
- size_t size() const { return _end - _begin; }
- bool empty() const { return _begin == _end; }
- CharT& front() { return *_begin; }
- const CharT& front() const { return *_begin; }
+ CharT* data() const noexcept { return _begin; }
+ size_t size() const noexcept { return _end - _begin; }
+ bool empty() const noexcept { return _begin == _end; }
+ CharT& front() noexcept { return *_begin; }
+ const CharT& front() const noexcept { return *_begin; }

- void remove_prefix(size_t n) {
+ void remove_prefix(size_t n) noexcept {
_begin += n;
}
- void remove_suffix(size_t n) {
+ void remove_suffix(size_t n) noexcept {
_end -= n;
}

--
2.31.1

[Benny Halevy]

Added assertions about access within allowed boundaries.
With those, mark the respective functions noexcept.

This is based on e.g. https://en.cppreference.com/w/cpp/string/basic_string_view/operator_at
that specifies: No bounds checking is performed: the behavior is undefined if pos >= size().
(i.e. it doesn't throw std::out_of_range, unlike the at() method)

Signed-off-by: Benny Halevy <bha...@scylladb.com>
---

utils/mutable_view.hh | 33 +++++++++++++++++++++++++++------
1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/utils/mutable_view.hh b/utils/mutable_view.hh
index 1f5078213..13e58c6c2 100644
--- a/utils/mutable_view.hh
+++ b/utils/mutable_view.hh
@@ -53,7 +53,10 @@ class basic_mutable_view {
return std::basic_string_view<CharT>(begin(), size());
}

- CharT& operator[](size_t idx) const { return _begin[idx]; }
+ CharT& operator[](size_t idx) const noexcept {
+ assert(idx < size());
+ return _begin[idx];

+ }

iterator begin() const noexcept { return _begin; }

iterator end() const noexcept { return _end; }

@@ -61,17 +64,35 @@ class basic_mutable_view {

CharT* data() const noexcept { return _begin; }

size_t size() const noexcept { return _end - _begin; }

bool empty() const noexcept { return _begin == _end; }

- CharT& front() noexcept { return *_begin; }
- const CharT& front() const noexcept { return *_begin; }
+
+ CharT& front() noexcept {
+ assert(!empty());
+ return *_begin;
+ }
+

+ const CharT& front() const noexcept {

+ assert(!empty());
+ return *_begin;

+ }

void remove_prefix(size_t n) noexcept {

- _begin += n;
+ if (__builtin_expect(n <= size(), true)) {
+ _begin += n;
+ } else {
+ _begin = _end;
+ }

}
+
void remove_suffix(size_t n) noexcept {

- _end -= n;
+ if (__builtin_expect(n <= size(), true)) {
+ _end -= n;
+ } else {
+ _end = _begin;
+ }
}

- basic_mutable_view substr(size_t pos, size_t count) {
+ basic_mutable_view substr(size_t pos, size_t count) noexcept {
+ assert(pos <= size());
size_t n = std::min(count, (_end - _begin) - pos);
return basic_mutable_view{_begin + pos, n};
}
--
2.31.1

[Benny Halevy]

Modeled after https://en.cppreference.com/w/cpp/string/basic_string_view/at

Bounds checking is performed, exception of type std::out_of_range will be thrown on invalid access.

Signed-off-by: Benny Halevy <bha...@scylladb.com>
---

utils/mutable_view.hh | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/utils/mutable_view.hh b/utils/mutable_view.hh
index 13e58c6c2..2f580bb62 100644
--- a/utils/mutable_view.hh
+++ b/utils/mutable_view.hh
@@ -58,6 +58,13 @@ class basic_mutable_view {
return _begin[idx];
}

+ const CharT& at(size_t idx) const {
+ if (idx >= size()) {
+ throw std::out_of_range(format("basic_mutable_view::at: idx (which is {}) >= this->size() (which is {})", idx, size()));
+ }

+ return _begin[idx];
+ }
+
iterator begin() const noexcept { return _begin; }
iterator end() const noexcept { return _end; }

--
2.31.1

[Benny Halevy]

In preparation to defining both mutable and immutable views
using basic_view_base.

Signed-off-by: Benny Halevy <bha...@scylladb.com>
---

utils/fragment_range.hh | 3 +--
utils/mutable_view.hh | 59 ++++++++++++++++++++++++++---------------
2 files changed, 39 insertions(+), 23 deletions(-)

diff --git a/utils/fragment_range.hh b/utils/fragment_range.hh
index cbf19a3d0..32b08b911 100644
--- a/utils/fragment_range.hh
+++ b/utils/fragment_range.hh
@@ -31,8 +31,7 @@
#include "marshal_exception.hh"
#include "bytes.hh"
#include "utils/bit_cast.hh"
-
-enum class mutable_view { no, yes, };
+#include "utils/mutable_view.hh"

/// Fragmented buffer
///
diff --git a/utils/mutable_view.hh b/utils/mutable_view.hh
index 2f580bb62..5283f62c6 100644
--- a/utils/mutable_view.hh
+++ b/utils/mutable_view.hh
@@ -26,25 +26,34 @@

#include "seastarx.hh"

-template<typename CharT>
-class basic_mutable_view {
- CharT* _begin = nullptr;
- CharT* _end = nullptr;
+enum class mutable_view { no, yes, };
+
+template <typename CharT, mutable_view is_mutable>
+class basic_view_base {
public:
using value_type = CharT;
using pointer = CharT*;
- using iterator = CharT*;
- using const_iterator = CharT*;
-
- basic_mutable_view() = default;
+ using const_pointer = const CharT*;
+ using base_pointer = std::conditional_t<is_mutable == mutable_view::yes, pointer, const_pointer>;
+ using reference = CharT&;
+ using const_reference = const CharT&;
+ using base_reference = std::conditional_t<is_mutable == mutable_view::yes, reference, const_reference>;
+ using iterator = pointer;
+ using const_iterator = const_pointer;
+ using base_iterator = std::conditional_t<is_mutable == mutable_view::yes, iterator, const_iterator>;
+private:
+ base_pointer _begin = nullptr;
+ base_pointer _end = nullptr;
+public:
+ basic_view_base() = default;

- template<typename U, U N>
- basic_mutable_view(basic_sstring<CharT, U, N>& str) noexcept
+ template <typename U, U N, bool NulTerminate>
+ basic_view_base(basic_sstring<CharT, U, N, NulTerminate>& str) noexcept
: _begin(str.begin())
, _end(str.end())
{ }

- basic_mutable_view(CharT* ptr, size_t length) noexcept
+ basic_view_base(base_pointer ptr, size_t length) noexcept

: _begin(ptr)
, _end(ptr + length)
{ }

@@ -53,31 +62,36 @@ class basic_mutable_view {

return std::basic_string_view<CharT>(begin(), size());
}

- CharT& operator[](size_t idx) const noexcept {
+ base_reference operator[](size_t idx) const noexcept {
assert(idx < size());
return _begin[idx];
}

- const CharT& at(size_t idx) const {
+ const_reference at(size_t idx) const {
if (idx >= size()) {
- throw std::out_of_range(format("basic_mutable_view::at: idx (which is {}) >= this->size() (which is {})", idx, size()));
+ throw std::out_of_range(format("basic_view_base::at: idx (which is {}) >= this->size() (which is {})", idx, size()));
}
return _begin[idx];
}

- iterator begin() const noexcept { return _begin; }
- iterator end() const noexcept { return _end; }
+ base_iterator begin() const noexcept { return _begin; }
+ base_iterator end() const noexcept { return _end; }

- CharT* data() const noexcept { return _begin; }
+ const_iterator cbegin() const noexcept { return _begin; }
+ const_iterator cend() const noexcept { return _end; }
+
+ base_pointer data() const noexcept { return _begin; }

size_t size() const noexcept { return _end - _begin; }
bool empty() const noexcept { return _begin == _end; }

- CharT& front() noexcept {

+ template <typename>
+ requires ( is_mutable == mutable_view::yes )
+ reference front() noexcept {
assert(!empty());

return *_begin;
}

- const CharT& front() const noexcept {

+ const_reference front() const noexcept {
assert(!empty());
return *_begin;
}
@@ -98,9 +112,12 @@ class basic_mutable_view {
}
}

- basic_mutable_view substr(size_t pos, size_t count) noexcept {
+ basic_view_base substr(size_t pos, size_t count) noexcept {

assert(pos <= size());
size_t n = std::min(count, (_end - _begin) - pos);

- return basic_mutable_view{_begin + pos, n};
+ return basic_view_base{_begin + pos, n};
}
};
+
+template <typename CharT>
+using basic_mutable_view = basic_view_base<CharT, mutable_view::yes>;
--
2.31.1

[Benny Halevy]

Signed-off-by: Benny Halevy <bha...@scylladb.com>
---

bytes.cc | 8 ++---
bytes.hh | 18 ++++++++---
utils/mutable_view.hh | 75 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 92 insertions(+), 9 deletions(-)

diff --git a/bytes.cc b/bytes.cc
index dba49f55b..1ad98b67a 100644
--- a/bytes.cc
+++ b/bytes.cc
@@ -82,10 +82,6 @@ sstring to_hex(const bytes_opt& b) {
return !b ? "null" : to_hex(*b);
}

-std::ostream& operator<<(std::ostream& os, const bytes& b) {
- return os << to_hex(b);
-}
-
std::ostream& operator<<(std::ostream& os, const bytes_opt& b) {
if (b) {
return os << *b;
@@ -95,6 +91,10 @@ std::ostream& operator<<(std::ostream& os, const bytes_opt& b) {

namespace std {

+std::ostream& operator<<(std::ostream& os, const bytes& b) {
+ return os << to_hex(b);
+}
+
std::ostream& operator<<(std::ostream& os, const bytes_view& b) {
return os << to_hex(b);
}
diff --git a/bytes.hh b/bytes.hh
index 35730c194..f1cf109b9 100644
--- a/bytes.hh
+++ b/bytes.hh
@@ -30,9 +30,10 @@
#include "utils/mutable_view.hh"
#include <xxhash.h>

-using bytes = basic_sstring<int8_t, uint32_t, 31, false>;
-using bytes_view = std::basic_string_view<int8_t>;
-using bytes_mutable_view = basic_mutable_view<bytes_view::value_type>;
+using bytes_char_type = int8_t;
+using bytes = basic_sstring<bytes_char_type, uint32_t, 31, false>;
+using bytes_view = basic_view<bytes_char_type>;
+using bytes_mutable_view = basic_mutable_view<bytes_char_type>;
using bytes_opt = std::optional<bytes>;
using sstring_view = std::string_view;

@@ -45,7 +46,7 @@ inline sstring_view to_sstring_view(bytes_view view) {
}

inline bytes_view to_bytes_view(sstring_view view) {
- return {reinterpret_cast<const int8_t*>(view.data()), view.size()};
+ return bytes_view(reinterpret_cast<const bytes_char_type*>(view.data()), view.size());
}

struct fmt_hex {
@@ -60,12 +61,12 @@ sstring to_hex(bytes_view b);
sstring to_hex(const bytes& b);
sstring to_hex(const bytes_opt& b);

-std::ostream& operator<<(std::ostream& os, const bytes& b);
std::ostream& operator<<(std::ostream& os, const bytes_opt& b);

namespace std {

// Must be in std:: namespace, or ADL fails
+std::ostream& operator<<(std::ostream& os, const bytes& b);
std::ostream& operator<<(std::ostream& os, const bytes_view& b);

}
@@ -110,6 +111,13 @@ struct hash<bytes_view> {
return h.finalize();
}
};
+
+template <>
+struct hash<bytes> {
+ size_t operator()(bytes b) const {
+ return std::hash<bytes_view>()(bytes_view(b));
+ }
+};
} // namespace std

inline int32_t compare_unsigned(bytes_view v1, bytes_view v2) {
diff --git a/utils/mutable_view.hh b/utils/mutable_view.hh
index 5283f62c6..894422b36 100644
--- a/utils/mutable_view.hh
+++ b/utils/mutable_view.hh
@@ -41,6 +41,9 @@ class basic_view_base {
using iterator = pointer;
using const_iterator = const_pointer;

using base_iterator = std::conditional_t<is_mutable == mutable_view::yes, iterator, const_iterator>;

+ using reverse_iterator = std::reverse_iterator<iterator>;
+ using const_reverse_iterator = std::reverse_iterator<const_iterator>;
+ using base_reverse_iterator = std::conditional_t<is_mutable == mutable_view::yes, reverse_iterator, const_reverse_iterator>;
private:
base_pointer _begin = nullptr;
base_pointer _end = nullptr;
@@ -48,20 +51,59 @@ class basic_view_base {
basic_view_base() = default;

template <typename U, U N, bool NulTerminate>

+ requires ( is_mutable == mutable_view::no )
+ basic_view_base(const basic_sstring<CharT, U, N, NulTerminate>& str) noexcept
+ : _begin(str.begin())
+ , _end(str.end())
+ { }
+

+ template <typename U, U N, bool NulTerminate>

+ requires ( is_mutable == mutable_view::yes )

basic_view_base(basic_sstring<CharT, U, N, NulTerminate>& str) noexcept
: _begin(str.begin())
, _end(str.end())
{ }

+ basic_view_base(std::basic_string_view<CharT> str) noexcept
+ : _begin(str.begin())
+ , _end(str.end())
+ { }

+
basic_view_base(base_pointer ptr, size_t length) noexcept
: _begin(ptr)
, _end(ptr + length)
{ }

+ template <typename>
+ requires ( is_mutable == mutable_view::no )
+ basic_view_base(const basic_view_base<CharT, mutable_view::yes>& x) noexcept
+ : _begin(x.begin())
+ , _end(x.end())
+ { }
+
+ template <typename>
+ requires ( is_mutable == mutable_view::no )
+ basic_view_base operator=(const basic_view_base<CharT, mutable_view::yes>& x) noexcept {
+ new (this) basic_view_base(x);
+ return *this;
+ }
+
operator std::basic_string_view<CharT>() const noexcept {

return std::basic_string_view<CharT>(begin(), size());
}

+ bool operator==(const basic_view_base& x) const noexcept {
+ return size() == x.size() && std::equal(begin(), end(), x.begin());
+ }
+
+ bool operator!=(const basic_view_base& x) const noexcept {
+ return !operator==(x);
+ }
+
+ constexpr auto operator<=>(const basic_view_base& x) const noexcept {
+ return std::basic_string_view<CharT>(*this) <=> std::basic_string_view<CharT>(x);
+ }

+
base_reference operator[](size_t idx) const noexcept {
assert(idx < size());
return _begin[idx];

@@ -80,8 +122,15 @@ class basic_view_base {

const_iterator cbegin() const noexcept { return _begin; }

const_iterator cend() const noexcept { return _end; }

+ base_reverse_iterator rbegin() const noexcept { return std::make_reverse_iterator<base_iterator>(end()); }
+ base_reverse_iterator rend() const noexcept { return std::make_reverse_iterator<base_iterator>(begin()); }
+
+ const_reverse_iterator crbegin() const noexcept { return std::make_reverse_iterator<const_iterator>(end()); }
+ const_reverse_iterator crend() const noexcept { return std::make_reverse_iterator<const_iterator>(begin()); }

+
base_pointer data() const noexcept { return _begin; }
size_t size() const noexcept { return _end - _begin; }

+ size_t length() const noexcept { return size(); }

bool empty() const noexcept { return _begin == _end; }

template <typename>
@@ -96,6 +145,18 @@ class basic_view_base {
return *_begin;

}

+ template <typename>
+ requires ( is_mutable == mutable_view::yes )

+ reference back() noexcept {
+ assert(!empty());
+ return *(_end - 1);
+ }
+
+ const_reference back() const noexcept {
+ assert(!empty());
+ return *(_end - 1);
+ }

+
void remove_prefix(size_t n) noexcept {

if (__builtin_expect(n <= size(), true)) {

_begin += n;
@@ -119,5 +180,19 @@ class basic_view_base {
}
};

+namespace std {

+
+template <typename CharT, mutable_view is_mutable>

+struct hash<basic_view_base<CharT, is_mutable>> {
+ size_t operator()(const basic_view_base<CharT, is_mutable>& v) const {
+ return std::hash<std::basic_string_view<CharT>>()(v);
+ }
+};
+
+}
+
template <typename CharT>

using basic_mutable_view = basic_view_base<CharT, mutable_view::yes>;
+
+template <typename CharT>

+using basic_view = basic_view_base<CharT, mutable_view::no>;
--
2.31.1

[Avi Kivity]

On 13/05/2021 13.50, Benny Halevy wrote:
> Currently bytes_mutable_view is defined in scylla
> while bytes_view uses std::basic_string_view<int8_t>.
>
> Both lack cheks for access out of range and may facilitate
> undefined behavior.
>
> This series hardens bytes_mutable_view and bytes_vies
> by adding asserts checking access within the view boundaries
> and marking most access functions noexcept. A common
> template - `basic_view_base` - is used by both
> to share the low-level logic.
>
> A possibly-throwing `at()` method was added.
> It checks the provided index and throws std::out_of_range
> if needed.
>
> perf_simple_query:
>
> Before:
> median 182633.31 tps ( 87.1 allocs/op, 20.1 tasks/op, 53727 insns/op)
> median absolute deviation: 1094.85
> maximum: 183728.16
> minimum: 164361.01
>
> After:
> median 187516.88 tps ( 87.1 allocs/op, 20.1 tasks/op, 54160 insns/op)
> median absolute deviation: 572.40
> maximum: 188089.28
> minimum: 186214.28

Looks like it's increasing the instruction count by 1%. Is it worth it?

[Benny Halevy]

First, I apologize for sending to the wrong mailing list.

Despite the increase in instructions tps actually seem to have improved and it's consistent in several runs I do see a potential in converting from std::basic_string_view to our own data structure.

Ideally, those asserts could be enabled only in non-release builds and we could benefit from both worlds.

[Avi Kivity]

You're probably  measuring the temperature of the CPU. Try fixing the CPU frequency to something low.

To what do you attribute the improvement?

I do see a potential in converting from std::basic_string_view to our own data structure.

Where would it help? basic_string_view is trivial.

Ideally, those asserts could be enabled only in non-release builds and we could benefit from both worlds.

Yes.

[Benny Halevy]

I'll try, although I started at the branch,

then checked out master and the branch HEAD again.

To what do you attribute the improvement?

I don't know.

I do see a potential in converting from std::basic_string_view to our own data structure.

Where would it help? basic_string_view is trivial.

I don't have an explanation for the numbers I measured.

I'll try again with a completely idle machine otherwise and fixing the cpu frequency as you proposed.

[Benny Halevy]

Hmm, I reran pef_simple_query on an idle machine
after rebuilding from scratch, with cpufreq/scaling_governor
set to "performance" on all cpus.

The numbers for the assert enabled
still don't make sense. Maybe it's due to icache
effects because of different code placement?

command line: build/release/test/perf/perf_simple_query -c1 --default-log-level=error --duration=30 --random-seed=3264522186

Before:
median 195981.92 tps ( 87.1 allocs/op, 20.1 tasks/op, 53756 insns/op)

My branch with assert:
median 196673.00 tps ( 87.1 allocs/op, 20.1 tasks/op, 54208 insns/op)

My branch without assert:
median 188886.76 tps ( 87.1 allocs/op, 20.1 tasks/op, 54015 insns/op)

[Botond Dénes]

On Thu, 2021-05-13 at 13:50 +0300, Benny Halevy wrote:
> Currently bytes_mutable_view is defined in scylla
> while bytes_view uses std::basic_string_view<int8_t>.
>
> Both lack cheks for access out of range and may facilitate
> undefined behavior.

This is the standard way for C++ containers, none of the standard
containers have bounds checks. If this is an area where we are seeing
problems commonly, maybe we can add debug-only checks that are enabled
via a preprocessor switch, akin to the shared ptr debug checks.

[Avi Kivity]

On 14/05/2021 12.51, Benny Halevy wrote:
>
>>>
>>>
>>>> Ideally, those asserts could be enabled only in non-release builds and we could benefit from both worlds.
>>>>
>>> Yes.
>>>
> Hmm, I reran pef_simple_query on an idle machine
> after rebuilding from scratch, with cpufreq/scaling_governor
> set to "performance" on all cpus.

This is still thermally limited.


I use


    $ sudo cpupower frequency-set -g userspace
    $ sudo cpupower frequency-set -f 2000


to select a frequency that can always be achieved. Don't forget to set
it back afterwards.

> The numbers for the assert enabled
> still don't make sense. Maybe it's due to icache
> effects because of different code placement?
>
> command line: build/release/test/perf/perf_simple_query -c1 --default-log-level=error --duration=30 --random-seed=3264522186

I also use "--operations-per-shard 1000000 --task-quota-ms" to reduce
the effect of polling (when measuring instructions per op, not when
measuring raw performance).