Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Geos36 #636

Closed
wants to merge 7 commits into from
Closed

Geos36 #636

wants to merge 7 commits into from

Conversation

darkblue-b
Copy link

@darkblue-b darkblue-b commented Oct 28, 2016

this built and tests passed, with the following Ubuntu 1404 Trusty config :

osm2pgsql_geos36/build$ cmake .. -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=Debug -DBUILD_TESTS=ON
-- The C compiler identification is GNU 4.8.4
-- The CXX compiler identification is GNU 4.8.4
-- Check for working C compiler: /usr/bin/cc
-- Check for working C compiler: /usr/bin/cc -- works
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working CXX compiler: /usr/bin/c++
-- Check for working CXX compiler: /usr/bin/c++ -- works
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Building osm2pgsql 0.91.0-dev
-- Looking for include file termios.h
-- Looking for include file termios.h - found
-- Looking for include file libgen.h
-- Looking for include file libgen.h - found
-- Looking for include file unistd.h
-- Looking for include file unistd.h - found
-- Looking for lseek64
-- Looking for lseek64 - found
-- Looking for posix_fallocate
-- Looking for posix_fallocate - found
-- Looking for posix_fadvise
-- Looking for posix_fadvise - found
-- Looking for sync_file_range
-- Looking for sync_file_range - found
-- Looking for sys/types.h
-- Looking for sys/types.h - found
-- Looking for stdint.h
-- Looking for stdint.h - found
-- Looking for stddef.h
-- Looking for stddef.h - found
-- Check size of off_t
-- Check size of off_t - done
-- Found ZLIB: /usr/lib/x86_64-linux-gnu/libz.so (found version "1.2.8") 
-- Looking for include file pthread.h
-- Looking for include file pthread.h - found
-- Looking for pthread_create
-- Looking for pthread_create - not found
-- Looking for pthread_create in pthreads
-- Looking for pthread_create in pthreads - not found
-- Looking for pthread_create in pthread
-- Looking for pthread_create in pthread - found
-- Found Threads: TRUE  
-- Found EXPAT: /usr/lib/x86_64-linux-gnu/libexpat.so (found version "2.1.0") 
-- Found BZip2: /usr/lib/x86_64-linux-gnu/libbz2.so (found version "1.0.6") 
-- Looking for BZ2_bzCompressInit in /usr/lib/x86_64-linux-gnu/libbz2.so
-- Looking for BZ2_bzCompressInit in /usr/lib/x86_64-linux-gnu/libbz2.so - found
-- Found Osmium: /home/shared/srcs_i7d/osm2pgsql_geos36/contrib/libosmium  
-- Found Lua: /usr/lib/x86_64-linux-gnu/liblua5.2.so;/usr/lib/x86_64-linux-gnu/libm.so (found version "5.2.3") 
-- Boost version: 1.55.0
-- Found the following Boost libraries:
--   system
--   filesystem
-- Found PostgreSQL: /usr/lib/x86_64-linux-gnu/libpq.so (found version "9.6.1") 
Libraries used to build: /usr/lib/x86_64-linux-gnu/libboost_system.so/usr/lib/x86_64-linux-gnu/libboost_filesystem.so/usr/lib/x86_64-linux-gnu/libpq.so/usr/lib/x86_64-linux-gnu/libz.so-lpthread/usr/lib/x86_64-linux-gnu/libexpat.so/usr/lib/x86_64-linux-gnu/libbz2.so/usr/local/lib/libgeos.so/usr/lib/libproj.so/usr/lib/x86_64-linux-gnu/liblua5.2.so/usr/lib/x86_64-linux-gnu/libm.so

@joto
Copy link
Collaborator

joto commented Oct 28, 2016

Wouldn't it be simpler to just use GeometryFactory::getDefaultInstance() wherever the factory is needed instead of passing it around?

Does this still compile with GEOS 3.5 and smaller?

@pnorman
Copy link
Collaborator

pnorman commented Oct 28, 2016

Does this still compile with GEOS 3.5 and smaller?

Travis will check with 3.3.8, my main concerns are memory leaks in either version, thread safety, and performances.

@strk
Copy link
Contributor

strk commented Oct 28, 2016

getDefaultInstance is not thread-safe. You'll have to create a new factory locally if you need that.
But starting with geos-3.6 you are required to use the exposed static methods to create and destroy factories (not available in previous versions) so you'll need to write wrappers for that

@strk
Copy link
Contributor

strk commented Oct 28, 2016

getDefaultInstance definition is here:
https://github.com/libgeos/libgeos/blob/svn-3.5/src/geom/GeometryFactory.cpp#L728

At this stage the Factory has no state, so shouldn't be a big deal to use from multiple threads (beside taking care of not leaking one by calling getDefaultInstance at the same time by multiple threads). But in the future things may change.

The C-API of GEOS has a thread-safe interface, and you should still switch to it for API stability.

@ilovezfs
Copy link

any update here?

@pnorman
Copy link
Collaborator

pnorman commented Dec 31, 2016

ping @darkblue-b

@pnorman
Copy link
Collaborator

pnorman commented Mar 3, 2017

Fixed by #684

@pnorman pnorman closed this Mar 3, 2017
@ilovezfs ilovezfs mentioned this pull request Apr 28, 2017
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants