Skip to content

Commit 4ed6554

Browse files
trxcllnttrxcllnt
committed
Merge branch 'master' of https://github.com/apache/arrow into js-buffer-writer
2 parents f497f7a + 5bb3d85 commit 4ed6554

22 files changed

Lines changed: 374 additions & 140 deletions

‎.travis.yml‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ matrix:
136136
- if [ $ARROW_CI_INTEGRATION_AFFECTED != "1" ]; then exit; fi
137137
- $TRAVIS_BUILD_DIR/ci/travis_install_linux.sh
138138
- $TRAVIS_BUILD_DIR/ci/travis_install_clang_tools.sh
139-
- nvm install v10.1.0
139+
- nvm install 10.1
140140
- $TRAVIS_BUILD_DIR/ci/travis_before_script_js.sh
141141
- $TRAVIS_BUILD_DIR/ci/travis_before_script_cpp.sh
142142
script:
@@ -196,16 +196,17 @@ matrix:
196196
before_script:
197197
- if [ $ARROW_CI_RUST_AFFECTED != "1" ]; then exit; fi
198198
- pip install 'travis-cargo<0.2' --user && export PATH=$HOME/.local/bin:$PATH
199-
- cargo install --force cargo-travis && export PATH=$HOME/.cargo/bin:$PATH
199+
- export CARGO_TARGET_DIR=$TRAVIS_BUILD_DIR/target
200+
- cargo install cargo-travis || echo "Skipping cargo-travis installation as it already exists in cache"
201+
- export PATH=$HOME/.cargo/bin:$PATH
200202
script:
201203
- $TRAVIS_BUILD_DIR/ci/travis_script_rust.sh
202204
after_success:
203205
- pushd ${TRAVIS_BUILD_DIR}/rust
204206
# Run coverage for codecov.io
207+
- mkdir -p target/kcov
205208
- RUST_BACKTRACE=1 cargo coverage --verbose
206209
- bash <(curl -s https://codecov.io/bash) || echo "Codecov did not collect coverage reports"
207-
# Run coverage for coveralls.io
208-
#- cargo coveralls --verbose
209210

210211
after_failure:
211212
- COREFILE=$(find . -maxdepth 2 -name "core*" | head -n 1)

‎c_glib/arrow-glib/decimal.cpp‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -176,17 +176,30 @@ garrow_decimal128_to_string(GArrowDecimal128 *decimal)
176176
* garrow_decimal128_abs:
177177
* @decimal: A #GArrowDecimal128.
178178
*
179-
* Returns: (transfer full): The absolute value of the decimal.
179+
* Computes the absolute value of the @decimal destructively.
180180
*
181181
* Since: 0.10.0
182182
*/
183-
GArrowDecimal128 *
183+
void
184184
garrow_decimal128_abs(GArrowDecimal128 *decimal)
185185
{
186186
auto arrow_decimal = garrow_decimal128_get_raw(decimal);
187-
auto arrow_sub_decimal =
188-
std::make_shared<arrow::Decimal128>(arrow_decimal->Abs());
189-
return garrow_decimal128_new_raw(&arrow_sub_decimal);
187+
arrow_decimal->Abs();
188+
}
189+
190+
/**
191+
* garrow_decimal128_negate:
192+
* @decimal: A #GArrowDecimal128.
193+
*
194+
* Negate the current value of the @decimal destructively.
195+
*
196+
* Since: 0.10.0
197+
*/
198+
void
199+
garrow_decimal128_negate(GArrowDecimal128 *decimal)
200+
{
201+
auto arrow_decimal = garrow_decimal128_get_raw(decimal);
202+
arrow_decimal->Negate();
190203
}
191204

192205
G_END_DECLS

‎c_glib/arrow-glib/decimal.h‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ GArrowDecimal128 *garrow_decimal128_new_integer(const gint64 data);
4040
gchar *garrow_decimal128_to_string_scale(GArrowDecimal128 *decimal,
4141
gint32 scale);
4242
gchar *garrow_decimal128_to_string(GArrowDecimal128 *decimal);
43-
GArrowDecimal128 *garrow_decimal128_abs(GArrowDecimal128 *decimal);
43+
void garrow_decimal128_abs(GArrowDecimal128 *decimal);
44+
void garrow_decimal128_negate(GArrowDecimal128 *decimal);
4445

4546
G_END_DECLS

‎c_glib/test/test-decimal.rb‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,17 @@ def test_abs
3333
absolute_value = "23049223942343532412"
3434
negative_value = "-23049223942343532412"
3535
decimal = Arrow::Decimal128.new(negative_value)
36-
assert_equal(absolute_value, decimal.abs.to_s)
36+
decimal.abs
37+
assert_equal(absolute_value, decimal.to_s)
38+
end
39+
40+
def test_negate
41+
positive_value = "23049223942343532412"
42+
negative_value = "-23049223942343532412"
43+
decimal = Arrow::Decimal128.new(positive_value)
44+
decimal.negate
45+
assert_equal(negative_value, decimal.to_s)
46+
decimal.negate
47+
assert_equal(positive_value, decimal.to_s)
3748
end
3849
end

‎cpp/build-support/run_clang_format.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,6 @@
103103
if diff:
104104
# Print out the diff to stderr
105105
error = True
106-
print(",".join(map(str, map(type, diff))))
107106
sys.stderr.writelines(diff)
108107

109108
sys.exit(1 if error else 0)

‎cpp/src/plasma/common.cc‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717

1818
#include "plasma/common.h"
1919

20+
#include <limits>
21+
#include <mutex>
2022
#include <random>
2123

2224
#include "plasma/plasma_generated.h"
@@ -28,9 +30,15 @@ using arrow::Status;
2830
UniqueID UniqueID::from_random() {
2931
UniqueID id;
3032
uint8_t* data = id.mutable_data();
31-
std::random_device engine;
33+
// NOTE(pcm): The right way to do this is to have one std::mt19937 per
34+
// thread (using the thread_local keyword), but that's not supported on
35+
// older versions of macOS (see https://stackoverflow.com/a/29929949)
36+
static std::mutex mutex;
37+
std::lock_guard<std::mutex> lock(mutex);
38+
static std::mt19937 generator;
39+
std::uniform_int_distribution<uint32_t> dist(0, std::numeric_limits<uint8_t>::max());
3240
for (int i = 0; i < kUniqueIDSize; i++) {
33-
data[i] = static_cast<uint8_t>(engine());
41+
data[i] = static_cast<uint8_t>(dist(generator));
3442
}
3543
return id;
3644
}

‎cpp/src/plasma/plasma.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ extern "C" {
3030
void dlfree(void* mem);
3131
}
3232

33-
ObjectTableEntry::ObjectTableEntry() : pointer(nullptr) {}
33+
ObjectTableEntry::ObjectTableEntry() : pointer(nullptr), ref_count(0) {}
3434

3535
ObjectTableEntry::~ObjectTableEntry() {
3636
dlfree(pointer);

‎cpp/src/plasma/plasma.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,9 @@ struct ObjectTableEntry {
134134
/// IPC GPU handle to share with clients.
135135
std::shared_ptr<CudaIpcMemHandle> ipc_handle;
136136
#endif
137-
/// Set of clients currently using this object.
138-
std::unordered_set<Client*> clients;
137+
/// Number of clients currently using this object.
138+
int ref_count;
139+
139140
/// The state of the object, e.g., whether it is open or sealed.
140141
object_state state;
141142
/// The digest of the object. Used to see if two objects are the same.

‎cpp/src/plasma/store.cc‎

Lines changed: 42 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -124,21 +124,24 @@ const PlasmaStoreInfo* PlasmaStore::get_plasma_store_info() { return &store_info
124124

125125
// If this client is not already using the object, add the client to the
126126
// object's list of clients, otherwise do nothing.
127-
void PlasmaStore::add_client_to_object_clients(ObjectTableEntry* entry, Client* client) {
127+
void PlasmaStore::add_to_client_object_ids(ObjectTableEntry* entry, Client* client) {
128128
// Check if this client is already using the object.
129-
if (entry->clients.find(client) != entry->clients.end()) {
129+
if (client->object_ids.find(entry->object_id) != client->object_ids.end()) {
130130
return;
131131
}
132132
// If there are no other clients using this object, notify the eviction policy
133133
// that the object is being used.
134-
if (entry->clients.size() == 0) {
134+
if (entry->ref_count == 0) {
135135
// Tell the eviction policy that this object is being used.
136136
std::vector<ObjectID> objects_to_evict;
137137
eviction_policy_.begin_object_access(entry->object_id, &objects_to_evict);
138138
delete_objects(objects_to_evict);
139139
}
140-
// Add the client pointer to the list of clients using this object.
141-
entry->clients.insert(client);
140+
// Increase reference count.
141+
entry->ref_count++;
142+
143+
// Add object id to the list of object ids that this client is using.
144+
client->object_ids.insert(entry->object_id);
142145
}
143146

144147
// Create a new object buffer in the hash table.
@@ -225,11 +228,11 @@ int PlasmaStore::create_object(const ObjectID& object_id, int64_t data_size,
225228
result->metadata_size = metadata_size;
226229
result->device_num = device_num;
227230
// Notify the eviction policy that this object was created. This must be done
228-
// immediately before the call to add_client_to_object_clients so that the
231+
// immediately before the call to add_to_client_object_ids so that the
229232
// eviction policy does not have an opportunity to evict the object.
230233
eviction_policy_.object_created(object_id);
231234
// Record that this client is using this object.
232-
add_client_to_object_clients(store_info_.objects[object_id].get(), client);
235+
add_to_client_object_ids(store_info_.objects[object_id].get(), client);
233236
return PlasmaError_OK;
234237
}
235238

@@ -324,7 +327,7 @@ void PlasmaStore::update_object_get_requests(const ObjectID& object_id) {
324327
get_req->num_satisfied += 1;
325328
// Record the fact that this client will be using this object and will
326329
// be responsible for releasing this object.
327-
add_client_to_object_clients(entry, get_req->client);
330+
add_to_client_object_ids(entry, get_req->client);
328331

329332
// If this get request is done, reply to the client.
330333
if (get_req->num_satisfied == get_req->num_objects_to_wait_for) {
@@ -358,7 +361,7 @@ void PlasmaStore::process_get_request(Client* client,
358361
get_req->num_satisfied += 1;
359362
// If necessary, record that this client is using this object. In the case
360363
// where entry == NULL, this will be called from seal_object.
361-
add_client_to_object_clients(entry, client);
364+
add_to_client_object_ids(entry, client);
362365
} else {
363366
// Add a placeholder plasma object to the get request to indicate that the
364367
// object is not present. This will be parsed by the client. We set the
@@ -383,14 +386,16 @@ void PlasmaStore::process_get_request(Client* client,
383386
}
384387
}
385388

386-
int PlasmaStore::remove_client_from_object_clients(ObjectTableEntry* entry,
387-
Client* client) {
388-
auto it = entry->clients.find(client);
389-
if (it != entry->clients.end()) {
390-
entry->clients.erase(it);
389+
int PlasmaStore::remove_from_client_object_ids(ObjectTableEntry* entry, Client* client) {
390+
auto it = client->object_ids.find(entry->object_id);
391+
if (it != client->object_ids.end()) {
392+
client->object_ids.erase(it);
393+
// Decrease reference count.
394+
entry->ref_count--;
395+
391396
// If no more clients are using this object, notify the eviction policy
392397
// that the object is no longer being used.
393-
if (entry->clients.size() == 0) {
398+
if (entry->ref_count == 0) {
394399
// Tell the eviction policy that this object is no longer being used.
395400
std::vector<ObjectID> objects_to_evict;
396401
eviction_policy_.end_object_access(entry->object_id, &objects_to_evict);
@@ -408,7 +413,7 @@ void PlasmaStore::release_object(const ObjectID& object_id, Client* client) {
408413
auto entry = get_object_table_entry(&store_info_, object_id);
409414
ARROW_CHECK(entry != NULL);
410415
// Remove the client from the object's array of clients.
411-
ARROW_CHECK(remove_client_from_object_clients(entry, client) == 1);
416+
ARROW_CHECK(remove_from_client_object_ids(entry, client) == 1);
412417
}
413418

414419
// Check if an object is present.
@@ -439,8 +444,8 @@ int PlasmaStore::abort_object(const ObjectID& object_id, Client* client) {
439444
ARROW_CHECK(entry != NULL) << "To abort an object it must be in the object table.";
440445
ARROW_CHECK(entry->state != PLASMA_SEALED)
441446
<< "To abort an object it must not have been sealed.";
442-
auto it = entry->clients.find(client);
443-
if (it == entry->clients.end()) {
447+
auto it = client->object_ids.find(object_id);
448+
if (it == client->object_ids.end()) {
444449
// If the client requesting the abort is not the creator, do not
445450
// perform the abort.
446451
return 0;
@@ -466,7 +471,7 @@ int PlasmaStore::delete_object(ObjectID& object_id) {
466471
return PlasmaError_ObjectNotSealed;
467472
}
468473

469-
if (entry->clients.size() != 0) {
474+
if (entry->ref_count != 0) {
470475
// To delete an object, there must be no clients currently using it.
471476
return PlasmaError_ObjectInUse;
472477
}
@@ -493,7 +498,7 @@ void PlasmaStore::delete_objects(const std::vector<ObjectID>& object_ids) {
493498
ARROW_CHECK(entry != NULL) << "To delete an object it must be in the object table.";
494499
ARROW_CHECK(entry->state == PLASMA_SEALED)
495500
<< "To delete an object it must have been sealed.";
496-
ARROW_CHECK(entry->clients.size() == 0)
501+
ARROW_CHECK(entry->ref_count == 0)
497502
<< "To delete an object, there must be no clients currently using it.";
498503
store_info_.objects.erase(object_id);
499504
// Inform all subscribers that the object has been deleted.
@@ -529,23 +534,27 @@ void PlasmaStore::disconnect_client(int client_fd) {
529534
// Close the socket.
530535
close(client_fd);
531536
ARROW_LOG(INFO) << "Disconnecting client on fd " << client_fd;
532-
// If this client was using any objects, remove it from the appropriate
533-
// lists.
534-
// TODO(swang): Avoid iteration through the object table.
537+
// Release all the objects that the client was using.
535538
auto client = it->second.get();
536-
std::vector<ObjectID> unsealed_objects;
537-
for (const auto& entry : store_info_.objects) {
538-
if (entry.second->state == PLASMA_SEALED) {
539-
remove_client_from_object_clients(entry.second.get(), client);
539+
std::vector<ObjectTableEntry*> sealed_objects;
540+
for (const auto& object_id : client->object_ids) {
541+
auto it = store_info_.objects.find(object_id);
542+
if (it == store_info_.objects.end()) {
543+
continue;
544+
}
545+
546+
if (it->second->state == PLASMA_SEALED) {
547+
// Add sealed objects to a temporary list of object IDs. Do not perform
548+
// the remove here, since it potentially modifies the object_ids table.
549+
sealed_objects.push_back(it->second.get());
540550
} else {
541-
// Add unsealed objects to a temporary list of object IDs. Do not perform
542-
// the abort here, since it potentially modifies the object table.
543-
unsealed_objects.push_back(entry.first);
551+
// Abort unsealed object.
552+
abort_object(it->first, client);
544553
}
545554
}
546-
// If the client was creating any objects, abort them.
547-
for (const auto& entry : unsealed_objects) {
548-
abort_object(entry, client);
555+
556+
for (const auto& entry : sealed_objects) {
557+
remove_from_client_object_ids(entry, client);
549558
}
550559

551560
// Note, the store may still attempt to send a message to the disconnected

‎cpp/src/plasma/store.h‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
#include <memory>
2323
#include <string>
2424
#include <unordered_map>
25+
#include <unordered_set>
2526
#include <vector>
2627

2728
#include "plasma/common.h"
@@ -46,6 +47,9 @@ struct Client {
4647

4748
/// The file descriptor used to communicate with the client.
4849
int fd;
50+
51+
/// Object ids that are used by this client.
52+
std::unordered_set<ObjectID, UniqueIDHasher> object_ids;
4953
};
5054

5155
class PlasmaStore {
@@ -164,13 +168,13 @@ class PlasmaStore {
164168
private:
165169
void push_notification(ObjectInfoT* object_notification);
166170

167-
void add_client_to_object_clients(ObjectTableEntry* entry, Client* client);
171+
void add_to_client_object_ids(ObjectTableEntry* entry, Client* client);
168172

169173
void return_from_get(GetRequest* get_req);
170174

171175
void update_object_get_requests(const ObjectID& object_id);
172176

173-
int remove_client_from_object_clients(ObjectTableEntry* entry, Client* client);
177+
int remove_from_client_object_ids(ObjectTableEntry* entry, Client* client);
174178

175179
/// Event loop of the plasma store.
176180
EventLoop* loop_;

0 commit comments

Comments
 (0)