Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,10 @@ if(MSVC)
set(ARROW_USE_GLOG OFF)
endif()

if(ARROW_JNI)
set(ARROW_BUILD_STATIC ON)
endif()

if(ARROW_ORC)
set(ARROW_WITH_LZ4 ON)
set(ARROW_WITH_SNAPPY ON)
Expand Down Expand Up @@ -729,6 +733,10 @@ if(ARROW_PARQUET)
endif()
endif()

if(ARROW_JNI)
add_subdirectory(src/jni)
endif()

if(ARROW_GANDIVA)
add_subdirectory(src/gandiva)
endif()
Expand Down
1 change: 1 addition & 0 deletions cpp/build-support/lint_cpp_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ def lint_file(path):
arrow/visitor_inline.h
gandiva/cache.h
gandiva/jni
jni/
test
internal''')

Expand Down
17 changes: 11 additions & 6 deletions cpp/cmake_modules/BuildUtils.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ function(ADD_ARROW_LIB LIB_NAME)
PRIVATE_INCLUDES
DEPENDENCIES
SHARED_INSTALL_INTERFACE_LIBS
STATIC_INSTALL_INTERFACE_LIBS)
STATIC_INSTALL_INTERFACE_LIBS
OUTPUT_PATH)
cmake_parse_arguments(ARG
"${options}"
"${one_value_args}"
Expand All @@ -164,6 +165,11 @@ function(ADD_ARROW_LIB LIB_NAME)
else()
set(BUILD_STATIC ${ARROW_BUILD_STATIC})
endif()
if(ARG_OUTPUT_PATH)
set(OUTPUT_PATH ${ARG_OUTPUT_PATH})
else()
set(OUTPUT_PATH ${BUILD_OUTPUT_ROOT_DIRECTORY})
endif()

if(WIN32 OR (CMAKE_GENERATOR STREQUAL Xcode))
# We need to compile C++ separately for each library kind (shared and static)
Expand Down Expand Up @@ -234,11 +240,11 @@ function(ADD_ARROW_LIB LIB_NAME)

set_target_properties(${LIB_NAME}_shared
PROPERTIES LIBRARY_OUTPUT_DIRECTORY
"${BUILD_OUTPUT_ROOT_DIRECTORY}"
"${OUTPUT_PATH}"
RUNTIME_OUTPUT_DIRECTORY
"${BUILD_OUTPUT_ROOT_DIRECTORY}"
"${OUTPUT_PATH}"
PDB_OUTPUT_DIRECTORY
"${BUILD_OUTPUT_ROOT_DIRECTORY}"
"${OUTPUT_PATH}"
LINK_FLAGS
"${ARG_SHARED_LINK_FLAGS}"
OUTPUT_NAME
Expand Down Expand Up @@ -313,8 +319,7 @@ function(ADD_ARROW_LIB LIB_NAME)
endif()

set_target_properties(${LIB_NAME}_static
PROPERTIES LIBRARY_OUTPUT_DIRECTORY
"${BUILD_OUTPUT_ROOT_DIRECTORY}" OUTPUT_NAME
PROPERTIES LIBRARY_OUTPUT_DIRECTORY "${OUTPUT_PATH}" OUTPUT_NAME
${LIB_NAME_STATIC})

if(ARG_STATIC_INSTALL_INTERFACE_LIBS)
Expand Down
2 changes: 2 additions & 0 deletions cpp/cmake_modules/DefineOptions.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,8 @@ if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")

define_option(ARROW_ORC "Build the Arrow ORC adapter" OFF)

define_option(ARROW_JNI "Build the Arrow JNI lib" OFF)

define_option(ARROW_TENSORFLOW "Build Arrow with TensorFlow support enabled" OFF)

define_option(ARROW_JEMALLOC "Build the Arrow jemalloc-based allocator" ON)
Expand Down
24 changes: 24 additions & 0 deletions cpp/src/jni/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

#
# arrow_jni
#

if(ARROW_ORC)
add_subdirectory(orc)
endif()
55 changes: 55 additions & 0 deletions cpp/src/jni/orc/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# Licensed to the Apache Software Foundation (ASF) under one
# or more contributor license agreements. See the NOTICE file
# distributed with this work for additional information
# regarding copyright ownership. The ASF licenses this file
# to you under the Apache License, Version 2.0 (the
# "License"); you may not use this file except in compliance
# with the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing,
# software distributed under the License is distributed on an
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.

#
# arrow_orc_jni
#

project(arrow_orc_jni)

cmake_minimum_required(VERSION 3.11)

find_package(JNI REQUIRED)

add_custom_target(arrow_orc_jni)

set(JNI_HEADERS_DIR "${CMAKE_CURRENT_BINARY_DIR}/generated")

add_subdirectory(../../../../java/adapter/orc ./java)

set(ARROW_BUILD_STATIC OFF)

add_arrow_lib(arrow_orc_jni
BUILD_SHARED
SOURCES
jni_wrapper.cpp
OUTPUTS
ARROW_ORC_JNI_LIBRARIES
SHARED_PRIVATE_LINK_LIBS
arrow_static
EXTRA_INCLUDES
${JNI_HEADERS_DIR}
PRIVATE_INCLUDES
${JNI_INCLUDE_DIRS}
${CMAKE_CURRENT_BINARY_DIR}
DEPENDENCIES
arrow_static
arrow_orc_java
OUTPUT_PATH
${CMAKE_CURRENT_BINARY_DIR})

add_dependencies(arrow_orc_jni ${ARROW_ORC_JNI_LIBRARIES})
80 changes: 80 additions & 0 deletions cpp/src/jni/orc/concurrent_map.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
*/

#ifndef JNI_ID_TO_MODULE_MAP_H
#define JNI_ID_TO_MODULE_MAP_H

#include <memory>
#include <mutex>
#include <unordered_map>
#include <utility>

#include "arrow/util/macros.h"

namespace arrow {
namespace jni {

/**
* An utility class that map module id to module pointers.
* @tparam Holder class of the object to hold.
*/
template <typename Holder>
class ConcurrentMap {
public:
ConcurrentMap() : module_id_(init_module_id_) {}

jlong Insert(Holder holder) {
std::lock_guard<std::mutex> lock(mtx_);
jlong result = module_id_++;
map_.insert(std::pair<jlong, Holder>(result, holder));
return result;
}

void Erase(jlong module_id) {
std::lock_guard<std::mutex> lock(mtx_);
map_.erase(module_id);
}

Holder Lookup(jlong module_id) {
std::lock_guard<std::mutex> lock(mtx_);
auto it = map_.find(module_id);
if (it != map_.end()) {
return it->second;
}
return NULLPTR;
}

void Clear() {
std::lock_guard<std::mutex> lock(mtx_);
map_.clear();
}

private:
// Initialize the module id starting value to a number greater than zero
// to allow for easier debugging of uninitialized java variables.
static constexpr int init_module_id_ = 4;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why the change back, the kInitiModuleId is what should be used for constants (also static shouldn't be required.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-static data member cannot be constexpr
and here is just changing the name to make it comply with Arrow C++ naming convention.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which naming convention are you referring to? As as I know constants are generally of the form kInitModuleId (non-static members follow the convention you have here).

Copy link
Author

@yuruiz yuruiz Jun 6, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a little confused about the naming convention in ARROW c++. In type.h I can find a lot static constexpr members follow my current convention like "type_id" so I thought this maybe a more consistent convention?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think at times were weren't consistent with our own conventions.

Theoretically we follow the google style guide with only a few exceptions

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At least for now let's follow the way like init_module_id_ to be consistent with rest of the codebase.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might have confused the point. Old code has both styles, New code should (and mostly does) follow the kInitModuleId style. At this point it would be counter-productive to change it back (I'd like to merge once CI passes) but in the future please use what is proscribed in the style guide.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good! Thank you!


int64_t module_id_;
std::mutex mtx_;
// map from module ids returned to Java and module pointers
std::unordered_map<jlong, Holder> map_;
};

} // namespace jni
} // namespace arrow

#endif // JNI_ID_TO_MODULE_MAP_H
Loading