Open Licenser opened 8 years ago
mmm i would be interrested to fix it. How could I test easily?
There are SmartOS vmware images that can get you started, or I'll try to get you a zone I'll check.
I will check. In the mean time can you try the patch below?
diff --git a/c_src/build_deps.sh b/c_src/build_deps.sh
index 107b6ae..1752301 100755
--- a/c_src/build_deps.sh
+++ b/c_src/build_deps.sh
@@ -67,7 +67,7 @@ case "$1" in
(cd snappy-$SNAPPY_VSN && $MAKE && $MAKE install)
- export CFLAGS="$CFLAGS -I $BASEDIR/system/include"
+ export CFLAGS="$CFLAGS -fno-omit-frame-pointer -I $BASEDIR/system/include"
export CXXFLAGS="$CXXFLAGS -I $BASEDIR/system/include"
export LDFLAGS="$LDFLAGS -L$BASEDIR/system/lib"
export LD_LIBRARY_PATH="$BASEDIR/system/lib:$LD_LIBRARY_PATH"
trying it out and preparing a test zone in the meantime ;)
no effect from what I can tell
ah forgot it for the unittests( the section above in the Makefile). Shouldn't be hard anyway, probably just a flag.
CC=/opt/local/gcc49/bin/gcc make
will run in a zone and fail as above, @ipalreadytaken has created a zone and I set it up with the packages for compilation will coordinate ip/pub key off channel.
Hi,
I tested on the node you gave me and went a little further in building erocksdb by doing the following:
1) set PATH to point to the gcc binaries: export PATH=$PATH:/opt/local/gcc49/bin
2) do the following diff:
[root@leo-test ~/erocksdb]# git diff
diff --git a/c_src/build_deps.sh b/c_src/build_deps.sh
index 71e36bc..222ca5c 100755
--- a/c_src/build_deps.sh
+++ b/c_src/build_deps.sh
@@ -8,7 +8,7 @@ if [ `uname -s` = 'SunOS' -a "${POSIX_SHELL}" != "true" ]; then
fi
unset POSIX_SHELL # clear it so if we invoke other scripts, they run as ksh as well
-ROCKSDB_VSN="rocksdb-3.11.2"
+ROCKSDB_VSN="rocksdb-4.1"
SNAPPY_VSN="1.1.1"
@@ -30,6 +30,18 @@ MAKE=${MAKE:-make}
# Changed "make" to $MAKE
+[ "$SYSTEM" ] || SYSTEM=`(uname -s) 2>/dev/null` || SYSTEM="unknown"
+
+CXX=
+CXXFLAGS="-fPIC"
+case "$SYSTEM" in
+ Solaris|SunOS)
+ CXX="gcc -Wno-return-type -Wno-sign-compare -Wno-unused-but-set-variable -std=c++11 -D_GLIBCXX_USE_C99 -D_GLIBCXX_USE_C99_MATH -D_GLIBCXX_USE_C99_MATH_TR1"
+ ;;
+ *)
+ ;;
+esac
+
case "$1" in
rm-deps)
rm -rf rocksdb system snappy-$SNAPPY_VSN
@@ -77,7 +89,7 @@ case "$1" in
(cd rocksdb && git checkout $ROCKSDB_VSN)
fi
if [ ! -f rocksdb/librocksdb.a ]; then
- (cd rocksdb && CXXFLAGS=-fPIC $MAKE static_lib)
+ (cd rocksdb && CXXFLAGS=$CXXFLAGS CXX=$CXX $MAKE static_lib)
fi
;;
esac
However it crashes with the following error:
CC utilities/geodb/geodb_impl.o
utilities/geodb/geodb_impl.cc: In member function 'rocksdb::Status rocksdb::GeoDBImpl::searchQuadIds(const rocksdb::GeoPosition&, double, std::vector<std::basic_string<char> >*)':
utilities/geodb/geodb_impl.cc:300:72: error: call of overloaded 'floor(unsigned int)' is ambiguous
int numberOfTilesAtMaxDepth = floor((bottomRight.x - topLeft.x) / 256);
^
utilities/geodb/geodb_impl.cc:300:72: note: candidates are:
In file included from /opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/math.h:47:0,
from /opt/local/gcc49/include/c++/cmath:44,
from /opt/local/gcc49/include/c++/random:38,
from /opt/local/gcc49/include/c++/bits/stl_algo.h:66,
from /opt/local/gcc49/include/c++/algorithm:62,
from ./utilities/geodb/geodb_impl.h:10,
from utilities/geodb/geodb_impl.cc:8:
/opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/iso/math_iso.h:214:21: note: long double std::floor(long double)
inline long double floor(long double __X) { return __floorl(__X); }
^
/opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/iso/math_iso.h:185:15: note: float std::floor(float)
inline float floor(float __X) { return __floorf(__X); }
^
/opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/iso/math_iso.h:98:15: note: double std::floor(double)
extern double floor __P((double));
^
utilities/geodb/geodb_impl.cc:301:61: error: call of overloaded 'log(int&)' is ambiguous
int zoomLevelsToRise = floor(::log(numberOfTilesAtMaxDepth) / ::log(2));
^
utilities/geodb/geodb_impl.cc:301:61: note: candidates are:
In file included from /opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/math.h:47:0,
from /opt/local/gcc49/include/c++/cmath:44,
from /opt/local/gcc49/include/c++/random:38,
from /opt/local/gcc49/include/c++/bits/stl_algo.h:66,
from /opt/local/gcc49/include/c++/algorithm:62,
from ./utilities/geodb/geodb_impl.h:10,
from utilities/geodb/geodb_impl.cc:8:
/opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/iso/math_iso.h:221:21: note: long double std::log(long double)
inline long double log(long double __X) { return __logl(__X); }
^
/opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/iso/math_iso.h:189:15: note: float std::log(float)
inline float log(float __X) { return __logf(__X); }
^
/opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/iso/math_iso.h:89:15: note: double std::log(double)
extern double log __P((double));
^
utilities/geodb/geodb_impl.cc:301:72: error: call of overloaded 'log(int)' is ambiguous
int zoomLevelsToRise = floor(::log(numberOfTilesAtMaxDepth) / ::log(2));
^
utilities/geodb/geodb_impl.cc:301:72: note: candidates are:
In file included from /opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/math.h:47:0,
from /opt/local/gcc49/include/c++/cmath:44,
from /opt/local/gcc49/include/c++/random:38,
from /opt/local/gcc49/include/c++/bits/stl_algo.h:66,
from /opt/local/gcc49/include/c++/algorithm:62,
from ./utilities/geodb/geodb_impl.h:10,
from utilities/geodb/geodb_impl.cc:8:
/opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/iso/math_iso.h:221:21: note: long double std::log(long double)
inline long double log(long double __X) { return __logl(__X); }
^
/opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/iso/math_iso.h:189:15: note: float std::log(float)
inline float log(float __X) { return __logf(__X); }
^
/opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/iso/math_iso.h:89:15: note: double std::log(double)
extern double log __P((double));
^
utilities/geodb/geodb_impl.cc: In static member function 'static rocksdb::GeoDBImpl::Tile rocksdb::GeoDBImpl::PixelToTile(const rocksdb::GeoDBImpl::Pixel&)':
utilities/geodb/geodb_impl.cc:356:43: error: call of overloaded 'floor(unsigned int)' is ambiguous
unsigned int tileX = floor(pixel.x / 256);
^
utilities/geodb/geodb_impl.cc:356:43: note: candidates are:
In file included from /opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/math.h:47:0,
from /opt/local/gcc49/include/c++/cmath:44,
from /opt/local/gcc49/include/c++/random:38,
from /opt/local/gcc49/include/c++/bits/stl_algo.h:66,
from /opt/local/gcc49/include/c++/algorithm:62,
from ./utilities/geodb/geodb_impl.h:10,
from utilities/geodb/geodb_impl.cc:8:
/opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/iso/math_iso.h:214:21: note: long double std::floor(long double)
inline long double floor(long double __X) { return __floorl(__X); }
^
/opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/iso/math_iso.h:185:15: note: float std::floor(float)
inline float floor(float __X) { return __floorf(__X); }
^
/opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/iso/math_iso.h:98:15: note: double std::floor(double)
extern double floor __P((double));
^
utilities/geodb/geodb_impl.cc:357:43: error: call of overloaded 'floor(unsigned int)' is ambiguous
unsigned int tileY = floor(pixel.y / 256);
^
utilities/geodb/geodb_impl.cc:357:43: note: candidates are:
In file included from /opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/math.h:47:0,
from /opt/local/gcc49/include/c++/cmath:44,
from /opt/local/gcc49/include/c++/random:38,
from /opt/local/gcc49/include/c++/bits/stl_algo.h:66,
from /opt/local/gcc49/include/c++/algorithm:62,
from ./utilities/geodb/geodb_impl.h:10,
from utilities/geodb/geodb_impl.cc:8:
/opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/iso/math_iso.h:214:21: note: long double std::floor(long double)
inline long double floor(long double __X) { return __floorl(__X); }
^
/opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/iso/math_iso.h:185:15: note: float std::floor(float)
inline float floor(float __X) { return __floorf(__X); }
^
/opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/iso/math_iso.h:98:15: note: double std::floor(double)
extern double floor __P((double));
^
Makefile:1022: recipe for target 'utilities/geodb/geodb_impl.o' failed
gmake[1]: *** [utilities/geodb/geodb_impl.o] Error 1
gmake[1]: Leaving directory '/root/erocksdb/c_src/rocksdb'
Makefile:12: recipe for target 'compile' failed
make: *** [compile] Error 2
I am not sure yet how to fix it. Any idea is welcome. Hopefully it will help :)
same error but I came to a simpler patch:
diff --git a/c_src/build_deps.sh b/c_src/build_deps.sh
index 71e36bc..69ffbf7 100755
--- a/c_src/build_deps.sh
+++ b/c_src/build_deps.sh
@@ -8,7 +8,7 @@ if [ `uname -s` = 'SunOS' -a "${POSIX_SHELL}" != "true" ]; then
fi
unset POSIX_SHELL # clear it so if we invoke other scripts, they run as ksh as well
-ROCKSDB_VSN="rocksdb-3.11.2"
+ROCKSDB_VSN="rocksdb-4.1"
SNAPPY_VSN="1.1.1"
@@ -30,6 +30,18 @@ MAKE=${MAKE:-make}
# Changed "make" to $MAKE
+[ "$SYSTEM" ] || SYSTEM=`(uname -s) 2>/dev/null` || SYSTEM="unknown"
+
+CXX=
+CXXFLAGS="-fPIC -fno-builtin-memcmp"
+case "$SYSTEM" in
+ Solaris|SunOS)
+ CXX="g++ -std=c++11 -D_GLIBCXX_USE_C99"
+ ;;
+ *)
+ ;;
+esac
+
case "$1" in
rm-deps)
rm -rf rocksdb system snappy-$SNAPPY_VSN
@@ -77,7 +89,7 @@ case "$1" in
(cd rocksdb && git checkout $ROCKSDB_VSN)
fi
if [ ! -f rocksdb/librocksdb.a ]; then
- (cd rocksdb && CXXFLAGS=-fPIC $MAKE static_lib)
+ (cd rocksdb && CXXFLAGS=$CXXFLAGS CXX=$CXX $MAKE static_lib)
fi
;;
esac
@benoitc thank you for tackling this issue! It seems that you almost get things done.
The remained error is only error: call of overloaded $FUNCTION is ambiguous
.
Those errors have happened because C++ overload resolution failed.
error: call of overloaded 'floor(unsigned int)' is ambiguous
int numberOfTilesAtMaxDepth = floor((bottomRight.x - topLeft.x) / 256);
^
utilities/geodb/geodb_impl.cc:300:72: note: candidates are:
In file included from
/opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/math.h:47:0,
opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/iso/math_iso.h:214:21: note: long double std::floor(long double)
inline long double floor(long double __X) { return __floorl(__X); }
^
/opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/iso/math_iso.h:185:15: note: float std::floor(float)
inline float floor(float __X) { return __floorf(__X); }
^
/opt/local/gcc49/lib/gcc/x86_64-sun-solaris2.11/4.9.2/include-fixed/iso/math_iso.h:98:15: note: double std::floor(double)
extern double floor __P((double));
In short, there is no function std::floor(unsigned int). there are only std::floor(long double), std::floor(float) and std::floor(double) defined in ISO C++ standard. So gcc49 on SmartOS is correct.
but some compilers on the other platforms have provided the below template function to handle thsese cases ( permit implicit conversion from any int types to double ) .
template<typename _Tp>
inline _GLIBCXX_CONSTEXPR
typename __gnu_cxx::__enable_if<__is_integer<_Tp>::__value,
double>::__type
floor(_Tp __x)
{ return __builtin_floor(__x); }
In those obtrusive compilers ,
template specialization with unsinged int
will output a below function.
inline _GLIBCXX_CONSTEXPR
double floor(unsigned int __x)
{ return __builtin_floor(__x); }
Since there is a function floor(unsinged int), Function overload will be resolved and works fine...
So there are 2 ways to solve this issue.
std::floor
calls and replace std::log(2)
with std::log(2f)
.IMHO, 1
should be the right direction.
Fix rocksdb/geodb to get rid of improper std::floor calls and replace std::log(2) with std::log(2f).
Yup, this is the right way. Would really appreciate a PR to fix this.
Very nice investigation, thanks!
@igorcanadi Sure! I'll PR later.
note: Filed on https://github.com/facebook/rocksdb/issues/956
note: Got to succeed in compiling and passing all the unit tests with gcc47 on smartos by applying the below patch and switch some errors into warnings and set _GLIBCXX_USE_SCHED_YIELD
to enable using std::this_thread::yield
std::map::emplace_hint
)diff --git a/db/version_set.cc b/db/version_set.cc
index 5198053..ef096e1 100644
--- a/db/version_set.cc
+++ b/db/version_set.cc
@@ -1924,7 +1924,8 @@ uint64_t VersionStorageInfo::EstimateLiveDataSize() const {
found_end = (lb == ranges.end());
if (found_end || internal_comparator_->Compare(
file->largest, (*lb).second->smallest) < 0) {
- ranges.emplace_hint(lb, &file->largest, file);
+ //ranges.emplace_hint(lb, &file->largest, file);
+ ranges[&file->largest] = file;
size += file->fd.file_size;
}
}
CXXFLAGS="-D _GLIBCXX_USE_SCHED_YIELD -Wno-error=sign-compare -Wno-error=conversion-null" make
I'll try gcc49 on smartos next week.
@mocchira maybe we could have some support for custom patches to the erocksdb build? All patches would be in a patches directory and applied one by one on the rocksdb source code depending on the platform. So we don't have to wait for a rocksdb release? Thoughts?
@benoitc right as a temporal solution until those patches merged into upstream. We'd like to have a patches directory for FreeBSD, SmartOS for now. So temporal patches are welcome!
@Licenser It seems https://github.com/facebook/rocksdb/pull/954/files has fixed this issue. Since this fix is still only on master, The latest erocksdb with the below patch may work.
diff --git a/c_src/build_deps.sh b/c_src/build_deps.sh
index 71e36bc..69ffbf7 100755
--- a/c_src/build_deps.sh
+++ b/c_src/build_deps.sh
-ROCKSDB_VSN="rocksdb-4.1"
+ROCKSDB_VSN="master"
As I and @benoitc said at the previous comment, We are considering to provide custom patches until each stable releases for rocksdb. please let us know if you want to try erocksdb before the next stable release for rocksdb.
Sorry for the late reply, I did give master + @benoitc patch a try it keeps failing in new places
gmake[1]: Entering directory '/root/erocksdb/c_src/rocksdb'
GEN util/build_version.cc
GEN util/build_version.cc
CC db/auto_roll_logger.o
In file included from ./util/statistics.h:14:0,
from ./util/stop_watch.h:8,
from ./util/perf_step_timer.h:9,
from ./util/iostats_context_imp.h:8,
from ./util/posix_logger.h:27,
from ./port/util_logger.h:18,
from ./db/auto_roll_logger.h:15,
from db/auto_roll_logger.cc:6:
./util/mutexlock.h: In member function 'void rocksdb::SpinMutex::lock()':
./util/mutexlock.h:106:9: error: 'yield' is not a member of 'std::this_thread'
Makefile:1252: recipe for target 'db/auto_roll_logger.o' failed
gmake[1]: *** [db/auto_roll_logger.o] Error 1
gmake[1]: Leaving directory '/root/erocksdb/c_src/rocksdb'
Makefile:12: recipe for target 'compile' failed
make: *** [compile] Error 2
I wish I could do more but c is a deep and dark rabbit hole for me
@Licenser Thank you for trying. you might also need to do this hack ( https://github.com/leo-project/erocksdb/issues/12#issuecomment-173854418 ) .
If that works for you, I'm going to commit those changes into build_deps.sh and rebar.config in order to compile properly on Smart OS.
Not even getting that far, it seems to be a issue with using 'yield'
I think I worked around that last issue all tests are passing now I'll try to re-plicate that!
@Licenser
Not even getting that far, it seems to be a issue with using 'yield'
Hmm.
In my Smart OS (the latest) on VMware with gcc47, gcc49,
As described at https://github.com/leo-project/erocksdb/issues/12#issuecomment-173854418 ,
-D _GLIBCXX_USE_SCHED_YIELD
compile option enable gcc4[7-9] to use std::this_thread::yield
and works fine.
Before trying to install erocksdb, I'd ask you to make a rocksdb alone.
$ cd ${rocksdb_root}
CXXFLAGS="-D _GLIBCXX_USE_SCHED_YIELD -Wno-error=sign-compare -Wno-error=conversion-null" make
@Licenser
I think I worked around that last issue all tests are passing now I'll try to re-plicate that!
Sorry for missing the comment. Good to hear that :)
The following error is printed when trying to compile erotcksdb under SmartOS: