Skip to content

Commit

Permalink
Refactor bitfield calculations for increased portability (#124)
Browse files Browse the repository at this point in the history
* nanoduration / nanoduration returns double instead of integer64

* incomplete solution for bitfield removal - NA is not yet handled correctly

* handle neg numbers

* remove '-mno-ms-bitfields' for Windows compilation

* Additional polish for bitfield change

* Roll micro version and date, update NEWS

---------

Co-authored-by: lsilvest <lsilvestri@ztsdb.org>
  • Loading branch information
eddelbuettel and lsilvest committed Jun 17, 2024
1 parent 0beec0c commit d3ec808
Show file tree
Hide file tree
Showing 11 changed files with 162 additions and 161 deletions.
30 changes: 30 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
@@ -1,3 +1,33 @@
2024-06-16 Dirk Eddelbuettel <edd@debian.org>

* DESCRIPTION (Version, Date): Roll minor version and date

* inst/include/nanotime/interval.hpp: Add extra braces
* src/interval.cpp: Updated output stream helper
* src/period.cpp: Idem

* src/Makevars (CXX_STD): Set C++17
* src/Makevars.win (CXX_STD): Idem
* src/Makevars.ucrt: Removed

2024-06-14 Leonardo Silvestri <lsilvestr@ztsdb.org>

* src/Makevars.win: Remove -mno-ms-bitfields

2024-06-13 Leonardo Silvestri <lsilvestr@ztsdb.org>

* inst/include/nanotime/interval.hpp: NA behavior for bitfield

2024-06-11 Leonardo Silvestri <lsilvestr@ztsdb.org>

* inst/include/nanotime/interval.hpp: Initial fix for bitfield
* inst/include/nanotime/period.hppL Idem
* src/interval.cpp: Idem

2024-06-09 Dirk Eddelbuettel <edd@debian.org>

* README.md: Use tinyverse.netlify.app for dependency badge

2024-05-24 Leonardo Silvestri <lsilvestr@ztsdb.org>

* R/nanoduration.R: duration divided by duration returns double
Expand Down
13 changes: 5 additions & 8 deletions DESCRIPTION
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
Package: nanotime
Type: Package
Title: Nanosecond-Resolution Time Support for R
Version: 0.3.7.5
Date: 2024-04-28
Version: 0.3.7.6
Date: 2024-06-16
Author: Dirk Eddelbuettel and Leonardo Silvestri
Maintainer: Dirk Eddelbuettel <edd@debian.org>
Description: Full 64-bit resolution date and time functionality with
Expand All @@ -16,10 +16,7 @@ License: GPL (>= 2)
URL: https://github.com/eddelbuettel/nanotime, https://eddelbuettel.github.io/nanotime/, https://dirk.eddelbuettel.com/code/nanotime.html
BugReports: https://github.com/eddelbuettel/nanotime/issues
RoxygenNote: 7.3.1
Collate:
'nanotime.R'
'nanoival.R'
'nanoduration.R'
'nanoperiod.R'
'RcppExports.R'
Collate: 'nanotime.R' 'nanoival.R' 'nanoduration.R' 'nanoperiod.R' 'RcppExports.R'
Encoding: UTF-8
NeedsCompilation: yes
Packaged: 2024-05-25 13:30:56 UTC; edd
2 changes: 2 additions & 0 deletions inst/NEWS.Rd
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
API function \code{Rf_asS4} (Leonardo in \ghpr{121} closing \ghit{120})
\item An \code{nanoduration} / \code{nanoduration} expression now returns
a double (Leonardo in \ghpr{122} closing \ghit{117})
\item Bitfield calculations no longer require an Windows-only compiler
switch (Leonardo in \ghpr{124})
}
}

Expand Down
163 changes: 78 additions & 85 deletions inst/include/nanotime/interval.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,114 +11,107 @@ namespace nanotime {

struct interval {

#ifdef WORDS_BIGENDIAN
constexpr interval() : s(0), sopen(0), e(0), eopen(0) { }
#else
constexpr interval() : sopen(0), s(0), eopen(0), e(0) { }
#endif
constexpr interval() : s_impl(0), e_impl(0) { }

interval(dtime s_p, dtime e_p, int sopen_p, int eopen_p)
#ifdef WORDS_BIGENDIAN
: s(s_p.time_since_epoch().count()), sopen(sopen_p),
e(e_p.time_since_epoch().count()), eopen(eopen_p) {
#else
: sopen(sopen_p), s(s_p.time_since_epoch().count()),
eopen(eopen_p), e(e_p.time_since_epoch().count()) {
#endif
interval(dtime s_p, dtime e_p, int sopen_p, int eopen_p) :
s_impl(s_p.time_since_epoch().count()),
e_impl(e_p.time_since_epoch().count())
{
if (sopen_p) {
s_impl |= std::int64_t{1} << 63;
}
if (eopen_p) {
e_impl |= std::int64_t{1} << 63;
}

// if any of the constructor parameters is NA, we construct an NA interval:
if (s_p.time_since_epoch() == duration::min() || e_p.time_since_epoch() == duration::min() ||
sopen_p == NA_INTEGER || eopen_p == NA_INTEGER) {
s = IVAL_NA;
e = IVAL_NA;
s_impl = IVAL_NA;
e_impl = IVAL_NA;
} else {
if (s_p.time_since_epoch().count() < IVAL_MIN || e_p.time_since_epoch().count() < IVAL_MIN) {
s = IVAL_NA;
e = IVAL_NA;
s_impl= IVAL_NA;
e_impl = IVAL_NA;
Rf_warning("NAs produced by time overflow (remember that interval times are coded with 63 bits)");
}
if (s_p.time_since_epoch().count() > IVAL_MAX || e_p.time_since_epoch().count() > IVAL_MAX) {
s = IVAL_NA;
e = IVAL_NA;
s_impl = IVAL_NA;
e_impl = IVAL_NA;
Rf_warning("NAs produced by time overflow (remember that interval times are coded with 63 bits)");
}
if (s > e) {
if (e() < s()) {
std::stringstream ss;
ss << "interval end (" << e << ") smaller than interval start (" << s << ")";
ss << "interval end (" << e() << ") smaller than interval start (" << s() << ")";
throw std::range_error(ss.str());
}
}
}

// interval(std::int64_t s_p, std::int64_t e_p, bool sopen_p, bool eopen_p)
// : sopen(sopen_p), s(s_p), eopen(eopen_p), e(e_p) {
// if (s > e) {
// std::stringstream ss;
// ss << "interval end (" << e << ") smaller than interval start (" << s << ")";
// throw std::range_error(ss.str());
// }
// }
dtime getStart() const { return dtime(duration(s())); }
dtime getEnd() const { return dtime(duration(e())); }

dtime getStart() const { return dtime(duration(s)); }
dtime getEnd() const { return dtime(duration(e)); }
bool isNA() const { return s == IVAL_NA; }

#ifdef WORDS_BIGENDIAN
std::int64_t s : 63;
bool sopen : 1; // encode if the interval's start boundary is open (true) or closed (false)
std::int64_t e : 63;
bool eopen : 1; // encode if the interval's end boundary is open (true) or closed (false)
#else
bool sopen : 1; // encode if the interval's start boundary is open (true) or closed (false)
std::int64_t s : 63;
bool eopen : 1; // encode if the interval's end boundary is open (true) or closed (false)
std::int64_t e : 63;
#endif
// below we do some bit gynmastic to use the uppermost bit to store whether the
// interval is opened or closed; bitfields would give us all that for free,
// but we can't rely upon them because of Windows:
bool sopen() const { return (s_impl & (std::int64_t{1} << 63)) != 0; }
bool eopen() const { return (e_impl & (std::int64_t{1} << 63)) != 0; }
static const std::int64_t bit64_compl = ~(std::int64_t{1} << 63);
static const std::int64_t bit63 = std::int64_t{1} << 62;
std::int64_t s() const { return s_impl & (bit64_compl | ((bit63 & s_impl) << 1)); }
std::int64_t e() const { return e_impl & (bit64_compl | ((bit63 & e_impl) << 1)); }

bool isNA() const { return s_impl == IVAL_NA; }

static const std::int64_t IVAL_MAX = 4611686018427387903LL;
static const std::int64_t IVAL_MIN = -4611686018427387903LL;
static const std::int64_t IVAL_NA = -4611686018427387904LL;
static const std::int64_t IVAL_NA = -9223372036854775807LL;

private:
std::int64_t s_impl; // start of ival; last bit encodes if boundary is open (1) or closed (0)
std::int64_t e_impl; // end of ival; last bit encodes if boundary is open (1) or closed (0)
};

// operators:

inline duration operator-(const interval& i1, const interval& i2) {
return duration(i1.s - i2.s);
return duration(i1.s() - i2.s());
}

inline bool operator==(const interval& i1, const interval& i2) {
return i1.s == i2.s && i1.e == i2.e && i1.sopen == i2.sopen &&
i1.eopen == i2.eopen;
return i1.s() == i2.s() && i1.e() == i2.e() && i1.sopen() == i2.sopen() &&
i1.eopen() == i2.eopen();
}

inline bool operator!=(const interval& i1, const interval& i2) {
return !(i1 == i2);
}

inline bool operator<=(const interval& i1, const interval& i2) {
if (i1.s < i2.s) return true;
if (i1.s == i2.s) {
if (!i1.sopen && i2.sopen) return true;
if (i1.sopen && !i2.sopen) return false;
// here we know that s1.sopen == s2.sopen
if (i1.e < i2.e) return true;
if (i1.e == i2.e) {
if (i1.eopen == i2.eopen) return true;
if (i1.eopen && !i2.eopen) return true;
if (i1.s() < i2.s()) return true;
if (i1.s() == i2.s()) {
if (!i1.sopen() && i2.sopen()) return true;
if (i1.sopen() && !i2.sopen()) return false;
// here we know that s1.sopen() == s2.sopen()
if (i1.e() < i2.e()) return true;
if (i1.e() == i2.e()) {
if (i1.eopen() == i2.eopen()) return true;
if (i1.eopen() && !i2.eopen()) return true;
}
}
return false;
}

inline bool operator<(const interval& i1, const interval& i2) {
if (i1.s < i2.s) return true;
if (i1.s == i2.s) {
if (!i1.sopen && i2.sopen) return true;
if (i1.sopen && !i2.sopen) return false;
// here we know that s1.sopen == s2.sopen
if (i1.e < i2.e) return true;
if (i1.e == i2.e) {
if (i1.eopen == i2.eopen) return false;
if (i1.eopen && !i2.eopen) return true;
if (i1.s() < i2.s()) return true;
if (i1.s() == i2.s()) {
if (!i1.sopen() && i2.sopen()) return true;
if (i1.sopen() && !i2.sopen()) return false;
// here we know that s1.sopen() == s2.sopen()
if (i1.e() < i2.e()) return true;
if (i1.e() == i2.e()) {
if (i1.eopen() == i2.eopen()) return false;
if (i1.eopen() && !i2.eopen()) return true;
}
}
return false;
Expand All @@ -133,30 +126,30 @@ namespace nanotime {
}

inline bool operator<(const dtime& i1, const interval& i2) {
if (i1.time_since_epoch().count() < i2.s) return true;
if (i1.time_since_epoch().count() == i2.s) return i2.sopen;
if (i1.time_since_epoch().count() < i2.s()) return true;
if (i1.time_since_epoch().count() == i2.s()) return i2.sopen();
return false;
}

inline bool operator>(const dtime& i1, const interval& i2) {
if (i1.time_since_epoch().count() > i2.e) return true;
if (i1.time_since_epoch().count() == i2.e) return i2.eopen;
if (i1.time_since_epoch().count() > i2.e()) return true;
if (i1.time_since_epoch().count() == i2.e()) return i2.eopen();
return false;
}

inline interval operator+(const interval& i, const duration d) {
// test duration is not > 63-bits, and after that the constructor can test for overflow:
return interval(i.getStart() + d, i.getEnd() + d, i.sopen, i.eopen);
return interval(i.getStart() + d, i.getEnd() + d, i.sopen(), i.eopen());
}

inline interval operator-(const interval& i, const duration d) {
// test duration is not > 63-bits, and after that the constructor can test for underflow:
return interval(i.getStart() - d, i.getEnd() - d, i.sopen, i.eopen);
return interval(i.getStart() - d, i.getEnd() - d, i.sopen(), i.eopen());
}

inline interval operator+(const duration d, const interval& i) {
// test duration is not > 63-bits, and after that the constructor can test for overflow:
return interval(i.getStart() + d, i.getEnd() + d, i.sopen, i.eopen);
return interval(i.getStart() + d, i.getEnd() + d, i.sopen(), i.eopen());
}

// interval components comparators:
Expand Down Expand Up @@ -195,10 +188,10 @@ namespace nanotime {

// interval comparators:
inline bool start_lt(const interval& i1, const interval& i2) {
return start_lt(i1.getStart(), i1.sopen, i2.getStart(), i2.sopen);
return start_lt(i1.getStart(), i1.sopen(), i2.getStart(), i2.sopen());
}
inline bool start_gt(const interval& i1, const interval& i2) {
return start_gt(i1.getStart(), i1.sopen, i2.getStart(), i2.sopen);
return start_gt(i1.getStart(), i1.sopen(), i2.getStart(), i2.sopen());
}
inline bool start_le(const interval& i1, const interval& i2) {
return !start_gt(i1,i2);
Expand All @@ -207,10 +200,10 @@ namespace nanotime {
return !start_lt(i1,i2);
}
inline bool end_lt(const interval& i1, const interval& i2) {
return end_lt(i1.getEnd(), i1.eopen, i2.getEnd(), i2.eopen);
return end_lt(i1.getEnd(), i1.eopen(), i2.getEnd(), i2.eopen());
}
inline bool end_gt(const interval& i1, const interval& i2) {
return end_gt(i1.getEnd(), i1.eopen, i2.getEnd(), i2.eopen);
return end_gt(i1.getEnd(), i1.eopen(), i2.getEnd(), i2.eopen());
}
inline bool end_le(const interval& i1, const interval& i2) {
return !end_gt(i1,i2);
Expand All @@ -222,10 +215,10 @@ namespace nanotime {
/// True if the end of 'i1' is smaller than the start of 'i2'. This
/// tests that 'i1' and 'i2' are disjoint and 'i2' is after 'i1'.
inline bool end_lt_start(const interval& i1, const interval& i2) {
return end_lt(i1.getEnd(), i1.eopen, i2.getStart(), i2.sopen);
return end_lt(i1.getEnd(), i1.eopen(), i2.getStart(), i2.sopen());
}
inline bool end_gt_start(const interval& i1, const interval& i2) {
return end_gt(i1.getEnd(), i1.eopen, i2.getStart(), i2.sopen);
return end_gt(i1.getEnd(), i1.eopen(), i2.getStart(), i2.sopen());
}
/// True if the end of 'i1' is greater or equal than the start of
/// 'i2'. This tests that 'i1' and 'i2' "touch" and that 'i2' is
Expand Down Expand Up @@ -277,10 +270,10 @@ namespace nanotime {

// interval comparators:
inline bool union_start_lt(const interval& i1, const interval& i2) {
return union_start_lt(i1.getStart(), i1.sopen, i2.getStart(), i2.sopen);
return union_start_lt(i1.getStart(), i1.sopen(), i2.getStart(), i2.sopen());
}
inline bool union_start_gt(const interval& i1, const interval& i2) {
return union_start_gt(i1.getStart(), i1.sopen, i2.getStart(), i2.sopen);
return union_start_gt(i1.getStart(), i1.sopen(), i2.getStart(), i2.sopen());
}
inline bool union_start_le(const interval& i1, const interval& i2) {
return !union_start_gt(i1,i2);
Expand All @@ -289,10 +282,10 @@ namespace nanotime {
return !union_start_lt(i1,i2);
}
inline bool union_end_lt(const interval& i1, const interval& i2) {
return union_end_lt(i1.getEnd(), i1.eopen, i2.getEnd(), i2.eopen);
return union_end_lt(i1.getEnd(), i1.eopen(), i2.getEnd(), i2.eopen());
}
inline bool union_end_gt(const interval& i1, const interval& i2) {
return union_end_gt(i1.getEnd(), i1.eopen, i2.getEnd(), i2.eopen);
return union_end_gt(i1.getEnd(), i1.eopen(), i2.getEnd(), i2.eopen());
}
inline bool union_end_le(const interval& i1, const interval& i2) {
return !union_end_gt(i1,i2);
Expand All @@ -304,10 +297,10 @@ namespace nanotime {
/// True if the end of 'i1' is smaller than the start of 'i2'. This
/// tests that 'i1' and 'i2' are disjoint and 'i2' is after 'i1'.
inline bool union_end_lt_start(const interval& i1, const interval& i2) {
return union_end_lt(i1.getEnd(), i1.eopen, i2.getStart(), i2.sopen);
return union_end_lt(i1.getEnd(), i1.eopen(), i2.getStart(), i2.sopen());
}
inline bool union_end_gt_start(const interval& i1, const interval& i2) {
return union_end_gt(i1.getEnd(), i1.eopen, i2.getStart(), i2.sopen);
return union_end_gt(i1.getEnd(), i1.eopen(), i2.getStart(), i2.sopen());
}
/// True if the end of 'i1' is greater or equal than the start of
/// 'i2'. This tests that 'i1' and 'i2' "touch" and that 'i2' is
Expand Down
4 changes: 2 additions & 2 deletions inst/include/nanotime/period.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,8 @@ namespace nanotime {


inline interval plus (const interval& i, const period& p, const std::string& z) {
return interval(plus(dtime{duration{i.s}}, p, z),
plus(dtime{duration{i.e}}, p, z), i.sopen, i.eopen);
return interval(plus(dtime{duration{i.s()}}, p, z),
plus(dtime{duration{i.e()}}, p, z), i.sopen(), i.eopen());
}
inline interval plus (const period& p, const interval& i, const std::string& z) {
return plus(i, p, z);
Expand Down
2 changes: 1 addition & 1 deletion man/nanoperiod.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions src/Makevars
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
## Compile as C++17
CXX_STD = CXX17

## We need headers from our package, the directory is not automatically included
PKG_CXXFLAGS = -I../inst/include
3 changes: 0 additions & 3 deletions src/Makevars.ucrt

This file was deleted.

4 changes: 3 additions & 1 deletion src/Makevars.win
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
## Compile as C++17
CXX_STD = CXX17

## We need headers from our package, the directory is not automatically included
PKG_CXXFLAGS = -I../inst/include -mno-ms-bitfields
PKG_CXXFLAGS = -I../inst/include
Loading

0 comments on commit d3ec808

Please sign in to comment.