eddelbuettel / rcppsimdjson

Rcpp Bindings for the 'simdjson' Header Library
117 stars 14 forks source link

Change Travis to build matrix over g++ 8, 9 and 10 #31

Closed eddelbuettel closed 4 years ago

eddelbuettel commented 4 years ago

See e.g. https://travis-ci.org/github/eddelbuettel/rcppsimdjson/builds/704937277

knapply commented 4 years ago

Did you plan to rebase feature/fparse immediately with this?

NM, got it!

eddelbuettel commented 4 years ago

Currently looking into that, and because you and I both touched .travis.yml it is less obvious than usually.

eddelbuettel commented 4 years ago

And adding -Xours does the trick (with the important note in the git rebase help that "ours" and "theirs" are swapped in a rebase. Who ever said git was intuitive? ;-)

eddelbuettel commented 4 years ago

So if you git reset --hard a few commits and then pull you should be all good again.

knapply commented 4 years ago

Ah okay. I needed to grab a fresh copy anyways, so I just re-did that and it's good to go. Thank you!

eddelbuettel commented 4 years ago

(BTW weird that gcc-7 pukes so reliably. Do we need to worry because "someone somewhere" will have that version?)

knapply commented 4 years ago

It's very weird. I'm just assuming someone will encounter it, so I have the fix.

For context, there are two APIs for simdjson: the default one w/ exceptions enabled and the one without. The key difference in usage is how you retrieve JSON element types.

The "no except" API works whether or not exceptions are enabled, so I used that to maximize future flexibility (the data's type/size is already confirmed at this point).

The fix I'm using (that I'll get in there ASAP) is to guard the matrix-builders (the only place it happens) so that the only-for-exceptions-enabled API is used if exceptions are enabled.

Then I just guard the switch that can turn exceptions off on our end so that gcc-7 can only compile w/ exceptions-enabled and thus can only ever touch the only-for-exceptions-enabled API.

The following code reproduces it...

// [[Rcpp::plugins(cpp17)]]
// [[Rcpp::depends(RcppSimdJson)]]

#define SIMDJSON_EXCEPTIONS 0  // 1: simdjson exceptions enabled

#define GUARD_GCC_7 0

#if GUARD_GCC_7 && __GNUC__ && __GNUC__ <= 7
#undef SIMDJSON_EXCEPTIONS // now simdjson.h will set it to 1 and enable exceptions
#define USE_EXCEPTIONS_API 1
#endif

#include <Rcpp.h>
#include <simdjson.h>
#include <simdjson.cpp>

// [[Rcpp::export]]
void test() {
  auto json = R"( [ [1.0,2.0], [3.0,4.0] ] )"_padded;

  simdjson::dom::parser parser;
  // we KNOW this is an array of arrays of doubles
  simdjson::dom::array array = parser.parse(json).get<simdjson::dom::array>().first;

#if SIMDJSON_EXCEPTIONS
  Rcpp::Rcout << "exceptions enabled" << std::endl;
#else
  Rcpp::Rcout << "exceptions disabled" << std::endl;
#endif

#if USE_EXCEPTIONS_API  // can only be used when exceptions are enabled
  Rcpp::Rcout << "exceptions-only API" << std::endl;

  for (simdjson::dom::array sub_array : array) {
    for (auto element : sub_array) {
      auto res = double(element);
    }
  }

#else  // can be used whether or not exceptions are enabled
  Rcpp::Rcout << "exceptions enabled or disabled API" << std::endl;

  for (auto sub_array : array) { // sub_array is still a simdjson::dom::element, compare w/ loop above
    for (auto element : sub_array.get<simdjson::dom::array>().first) {  // segfault
      auto res = element.get<double>().first;                                         
    }
  }

#endif
}

/*** R
test()
*/

This is where it gets even weirder: The segfault seems totally unrelated to R/Rcpp (at least I don't know of how it could be), but the equivalent straight-C++ has no such problem...

#define SIMDJSON_EXCEPTIONS 0

#include "simdjson.h"

int main() {
  auto json = "[[1,2],[3,4]]"_padded;

  simdjson::dom::parser parser;

  simdjson::dom::array array = parser.parse(json).get<simdjson::dom::array>().first;

#if SIMDJSON_EXCEPTIONS
  std::cout << "exceptions enabled" << std::endl;
#else
  std::cout << "exceptions disabled" << std::endl;
#endif

  for (auto sub_array : array) { 
    for (auto element : sub_array.get<simdjson::dom::array>().first) {
      auto res = element.get<double>().first;
      std::cout << res << std::endl;
    }
  }

  return 0;
}

g++-7 -std=c++17 -o test test.cpp simdjson.cpp && ./test

exceptions disabled
1
2
3
4
eddelbuettel commented 4 years ago

At least you have it under some control. So we can probably add gcc-7 easily enough to the matrix.

knapply commented 4 years ago

I'll add it. I'm wrapping up the fix for this, fparse()/fload(), and some housekeeping I've been meaning to get to.

knapply commented 4 years ago

Good to go: https://travis-ci.org/github/eddelbuettel/rcppsimdjson/builds/705026024

Any guesses as to what it could be?