ARROW-233: Add visibility macros, add static build option #100
Conversation
|
This must wait on apache/parquet-cpp#134 |
|
Looks ok, 👍 for the Code changes, waiting for Travis to be happy. |
wesm
force-pushed
the
ARROW-233
branch
2 times, most recently
from
56bd50b
to
3027692
Compare
| // valgrind to report a (spurious?) memory leak when needed in other shared | ||
| // libraries. The problem came up while adding explicit visibility to libarrow | ||
| // and libarrow_parquet | ||
| static TypePtr kBinaryValueType = TypePtr(new UInt8Type()); |
There was a problem hiding this comment.
@emkornfield this was a pretty thorny issue that took me a while to figure out (beyond my C++ DSO knowledge). The issue was that the static value_type_ was being used at compile time in translation units in libarrow_parquet, so something about the order of static initialization was causing a memory leak, or at least something valgrind thought was a memory leak. The problem didn't appear until symbol visibility became explicit
There was a problem hiding this comment.
:( I think I might have used this pattern a few other places. i'll do a scan and see if anything else pops up, submit the necessary PR
There was a problem hiding this comment.
So far so good; let's await the visibility patch to get green (still wrestling with builds, if you wouldn't mind a quick review of apache/parquet-cpp#135 it would help me)
|
Shoot, I think I accidentally clobbered |
|
If anything, it's pretty clear from this experience that we need to move the library artifact builds off the ASF repos, either to conda-forge or to an auxiliary repo where we can use docker + CircleCI. This might also help with setting up some more ad hoc integration testing that might not make sense in this repo |
- Hack around Travis CI inability to use its boost static libraries - Use parquet_shared name - More informative verbose test logs - Fix some gtest-1.7.0 crankiness - Fix a valgrind shared_ptr possible memory leak stemming from static variable referenced at compile-time in libarrow_parquet - Fix a bunch of compiler warnings in release builds Change-Id: I4f519751becaf6fbdbc0e2fe79938dec13876b30
|
Rebased after PARQUET-659...fingers crossed. |
|
Phew! @xhochy let me know when this one meets your approval |
|
+1, LGTM |
|
+1 🎉 |
Author: Uwe L. Korn <uwelk@xhochy.com> Closes apache#100 from xhochy/parquet-607 and squashes the following commits: 88fbfb5 [Uwe L. Korn] PARQUET-607: Public writer header
Author: Uwe L. Korn <uwelk@xhochy.com> Closes apache#100 from xhochy/parquet-607 and squashes the following commits: 88fbfb5 [Uwe L. Korn] PARQUET-607: Public writer header Change-Id: Iee0998f05d5a9625e17c3e9ae4b94a07ac5376a4
Author: Uwe L. Korn <uwelk@xhochy.com> Closes apache#100 from xhochy/parquet-607 and squashes the following commits: 88fbfb5 [Uwe L. Korn] PARQUET-607: Public writer header Change-Id: Iee0998f05d5a9625e17c3e9ae4b94a07ac5376a4
Author: Uwe L. Korn <uwelk@xhochy.com> Closes apache#100 from xhochy/parquet-607 and squashes the following commits: 88fbfb5 [Uwe L. Korn] PARQUET-607: Public writer header Change-Id: Iee0998f05d5a9625e17c3e9ae4b94a07ac5376a4
Author: Uwe L. Korn <uwelk@xhochy.com> Closes apache#100 from xhochy/parquet-607 and squashes the following commits: 88fbfb5 [Uwe L. Korn] PARQUET-607: Public writer header Change-Id: Iee0998f05d5a9625e17c3e9ae4b94a07ac5376a4
This also resolves ARROW-213. Builds off work done in PARQUET-489.
I inserted a hack to deal with the fast the boost libs in apt won't statically link properly. We'll deal with that some other time.