[GitHub] incubator-quickstep issue #100: Refactor bulk insert for SplitRowStore

[navsan]

Github user navsan commented on the issue:

https://github.com/apache/incubator-quickstep/pull/100
  
Sounds good! Thanks.

On Tue, Sep 20, 2016, 17:22 Marc S  wrote:

> @navsan  love the comments, very thorough.
>
> I updated the header of this PR to include benchmarks/testing info and
> will keep this in mind in the future.
>
> *Changes queued for this PR*
>
>- getting rid of TypedValue
>- moving some of the computations out of the inner loop and keeping a
>leaner set of running state variables
>- moving to a per-column info struct rather than a number of
>bitvectors and vectors
>
> I'm going to address these in an addendum commit. I think your suggestions
> are great.
>
> *Different PR*
>
>- getting rid of occupancy_bitmap_
>- moving the null_bitmap into the header as a single bitmap for the
>entire tuple storage subblock.
>
> I think these are valid issues (comments above for specifics) but that
> they can be the subject of another commit/PR in the pursuit of more 
concise
> PRs.
>
> *Another PR*
>
> Can you coalesce writes for contiguous columns in input/output? This helps
> speed up the common case of materialized join results where (almost)
> everything gets copied out anyway.
>
> Can you also add support for partialBulkInsert functions? See the commits
> in my branch for reference. It'll help us improve the join result
> materialization cost, as well as a few other operations.
>
> Both these points can be addressed but in a separate PR.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> 
,
> or mute the thread
> 

> .
>



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-quickstep issue #100: Refactor bulk insert for SplitRowStore

[cramja]

Github user cramja commented on the issue:

https://github.com/apache/incubator-quickstep/pull/100
  
@navsan love the comments, very thorough.

I updated the header of this PR to include benchmarks/testing info and will 
keep this in mind in the future.

**Changes queued for this PR**
* getting rid of TypedValue
* moving some of the computations out of the inner loop and keeping a 
leaner set of running state variables
* moving to a per-column info struct rather than a number of bitvectors and 
vectors

I'm going to address these in an addendum commit. I think your suggestions 
are great.

**Different PR**
* getting rid of occupancy_bitmap_
* moving the null_bitmap into the header as a single bitmap for the entire 
tuple storage subblock.

I think these are valid issues (comments above for specifics) but that they 
can be the subject of another commit/PR in the pursuit of more concise PRs.

**Another PR**
> Can you coalesce writes for contiguous columns in input/output? This 
helps speed up the common case of materialized join results where (almost) 
everything gets copied out anyway.

> Can you also add support for partialBulkInsert functions? See the commits 
in my branch for reference. It'll help us improve the join result 
materialization cost, as well as a few other operations.

Both these points can be addressed but in a separate PR. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[cramja]

Github user cramja commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79724504
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[cramja]

Github user cramja commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79723597
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[cramja]

Github user cramja commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79723161
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[cramja]

Github user cramja commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79723030
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[cramja]

Github user cramja commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79722961
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[cramja]

Github user cramja commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79722671
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[cramja]

Github user cramja commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79721974
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[cramja]

Github user cramja commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79721122
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[cramja]

Github user cramja commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79721010
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[cramja]

Github user cramja commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79720633
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep issue #98: QUICKSTEP-53 DateLit improvements

[hakanmemisoglu]

Github user hakanmemisoglu commented on the issue:

https://github.com/apache/incubator-quickstep/pull/98
  
I will close this PR since an issue in the compression code should be 
handled first.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-quickstep pull request #98: QUICKSTEP-53 DateLit improvements

[hakanmemisoglu]

Github user hakanmemisoglu closed the pull request at:

https://github.com/apache/incubator-quickstep/pull/98


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-quickstep pull request #90: Quickstep 28 29

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/incubator-quickstep/pull/90


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[navsan]

Github user navsan commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79701889
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[navsan]

Github user navsan commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79702096
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[navsan]

Github user navsan commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79702587
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[navsan]

Github user navsan commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79703426
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[navsan]

Github user navsan commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79702332
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[navsan]

Github user navsan commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79703207
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[navsan]

Github user navsan commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79702952
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep issue #100: Refactor bulk insert for SplitRowStore

[navsan]

Github user navsan commented on the issue:

https://github.com/apache/incubator-quickstep/pull/100
  
Finally, can you also add support for partialBulkInsert functions? See the 
commits in my branch for reference. It'll help us improve the join result 
materialization cost, as well as a few other operations. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-quickstep issue #100: Refactor bulk insert for SplitRowStore

[navsan]

Github user navsan commented on the issue:

https://github.com/apache/incubator-quickstep/pull/100
  
Also, always explain in the PR what tests/experiments you ran. 

- Did you confirm correctness of the code? How? Just the current test 
suite? TPCH queries?
- How did you determine the 2x performance improvement? What were the test 
parameters? What machine did you run this on?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-quickstep issue #100: Refactor bulk insert for SplitRowStore

[navsan]

Github user navsan commented on the issue:

https://github.com/apache/incubator-quickstep/pull/100
  
Please address the comments: mostly to do with performance improvements. 
Note that the objective here is not just to make SplitRowStore faster than it 
is today, but to get it to the point where we can drop PackedRowStore. We're 
not close to that right now, because we're doing far too much work in the inner 
loop. 

Also, 
- Consider getting rid of occupancy_bitmap_
- Consider moving the null_bitmap into the header as a single bitmap for 
the entire tuple storage subblock. 
- Consider getting rid of TypedValue 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[navsan]

Github user navsan commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79693915
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[navsan]

Github user navsan commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79695417
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -88,6 +88,67 @@ inline std::size_t 
CalculateVariableSizeWithRemappedAttributes(
   return total_size;
 }
 
+
+/**
+ * A struct which holds the offset information for a non-remapping insert
+ * operation
+ */
+struct BasicInsertInfo {
+  BasicInsertInfo(
+const CatalogRelationSchema &relation)
+: num_attrs_(relation.size()),
+  num_nullable_attrs_(relation.numNullableAttributes()),
+  max_var_length_(relation.getMaximumVariableByteLength()),
+  fixed_len_offset_(BitVector::BytesNeeded(num_nullable_attrs_)),
+  var_len_offset_(fixed_len_offset_ + relation.getFixedByteLength()),
+  is_variable_(num_attrs_),
+  is_nullable_(num_attrs_),
+  fixed_len_offsets_(num_attrs_),
+  fixed_len_sizes_(num_attrs_),
+  var_len_offsets_(num_attrs_) {
+attribute_id accessor_attr_id = 0;
+for (CatalogRelationSchema::const_iterator attr_it = relation.begin();
+ attr_it != relation.end();
+ ++attr_it, ++accessor_attr_id) {
+  DCHECK_EQ(accessor_attr_id, attr_it->getID());
+
+  const int nullable_idx = 
relation.getNullableAttributeIndex(accessor_attr_id);
+  const int variable_idx = 
relation.getVariableLengthAttributeIndex(accessor_attr_id);
+  is_nullable_.setBit(accessor_attr_id, nullable_idx != -1);
+
+  if (variable_idx == -1) {
+is_variable_.setBit(accessor_attr_id, false);
+fixed_len_offsets_[accessor_attr_id] = 
relation.getFixedLengthAttributeOffset(accessor_attr_id);
+fixed_len_sizes_[accessor_attr_id] = relation.getAttributeById(
+  accessor_attr_id)->getType().maximumByteLength();
+var_len_offsets_[accessor_attr_id] = -1;
+  } else {
+is_variable_.setBit(accessor_attr_id, true);
+fixed_len_offsets_[accessor_attr_id] = 0;
+fixed_len_sizes_[accessor_attr_id] = 0;
+var_len_offsets_[accessor_attr_id] = 
relation.getVariableLengthAttributeIndex(accessor_attr_id);
+  }
+}
+  }
+
+  std::size_t num_attrs_;
+  std::size_t num_nullable_attrs_;
+  std::size_t max_var_length_;
+
+  // byte offset from the beginning of a tuple to the first fixed length 
attribute
+  std::uint32_t fixed_len_offset_;
+  // byte offset from the beginning of a tuple to the first variable 
length offset/length pair
+  std::uint32_t var_len_offset_;
+
+  BitVector is_variable_;
--- End diff --

Packing these Booleans into a bitvector is needlessly expensive. Just use 
straight up 1-byte bools instead. We only have a few columns, so the space 
overhead is negligible. But doing so will avoid the complexity/cost of bit 
arithmetic to do lookups during insertion.
[This applies to both the bitmaps.]


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[navsan]

Github user navsan commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79694477
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[navsan]

Github user navsan commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79693696
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[navsan]

Github user navsan commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79692864
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -194,379 +257,125 @@ TupleStorageSubBlock::InsertResult 
SplitRowStoreTupleStorageSubBlock::insertTupl
 }
 
 tuple_id SplitRowStoreTupleStorageSubBlock::bulkInsertTuples(ValueAccessor 
*accessor) {
-  const tuple_id original_num_tuples = header_->num_tuples;
-  tuple_id pos = 0;
-
-  InvokeOnAnyValueAccessor(
-  accessor,
-  [&](auto *accessor) -> void {  // NOLINT(build/c++11)
-if (relation_.hasNullableAttributes()) {
-  if (relation_.isVariableLength()) {
-while (accessor->next()) {
-  // If packed, insert at the end of the slot array, otherwise 
find the
-  // first hole.
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  const std::size_t tuple_variable_bytes
-  = CalculateVariableSize(relation_, *accessor);
-  if (!this->spaceToInsert(pos, tuple_variable_bytes)) {
-accessor->previous();
-break;
-  }
-  // Allocate variable-length storage.
-  header_->variable_length_bytes_allocated += tuple_variable_bytes;
-
-  // Find the slot and locate its sub-structures.
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
-  std::uint32_t *variable_length_info_array = 
reinterpret_cast(
-  fixed_length_attr_storage + relation_.getFixedByteLength());
-  // Start writing variable-length data at the beginning of the 
newly
-  // allocated range.
-  std::uint32_t current_variable_position
-  = tuple_storage_bytes_ - 
header_->variable_length_bytes_allocated;
-
-  attribute_id accessor_attr_id = 0;
-  for (CatalogRelationSchema::const_iterator attr_it = 
relation_.begin();
-   attr_it != relation_.end();
-   ++attr_it, ++accessor_attr_id) {
-const int nullable_idx = 
relation_.getNullableAttributeIndex(attr_it->getID());
-const int variable_idx = 
relation_.getVariableLengthAttributeIndex(attr_it->getID());
-TypedValue 
attr_value(accessor->getTypedValue(accessor_attr_id));
-if ((nullable_idx != -1) && (attr_value.isNull())) {
-  // Set null bit and move on.
-  tuple_null_bitmap.setBit(nullable_idx, true);
-  continue;
-}
-if (variable_idx != -1) {
-  // Write offset and size into the slot, then copy the actual
-  // value into the variable-length storage region.
-  const std::size_t attr_size = attr_value.getDataSize();
-  variable_length_info_array[variable_idx << 1] = 
current_variable_position;
-  variable_length_info_array[(variable_idx << 1) + 1] = 
attr_size;
-  attr_value.copyInto(static_cast(tuple_storage_) + 
current_variable_position);
-  current_variable_position += attr_size;
-} else {
-  // Copy fixed-length value directly into the slot.
-  attr_value.copyInto(fixed_length_attr_storage
-  + 
relation_.getFixedLengthAttributeOffset(attr_it->getID()));
-}
-  }
-  // Update occupancy bitmap and header.
-  occupancy_bitmap_->setBit(pos, true);
-  ++(header_->num_tuples);
-  if (pos > header_->max_tid) {
-header_->max_tid = pos;
-  }
-}
-  } else {
-// Same as above, but skip variable-length checks.
-while (accessor->next()) {
-  pos = this->isPacked() ? header_->num_tuples
- : occupancy_bitmap_->firstZero(pos);
-  if (!this->spaceToInsert(pos, 0)) {
-accessor->previous();
-break;
-  }
-  void *tuple_slot = static_cast(tuple_storage_) + pos * 
tuple_slot_bytes_;
-  BitVector tuple_null_bitmap(tuple_slot,
-
relation_.numNullableAttributes());
-  tuple_null_bitmap.clear();
-  char *fixed_length_attr_storage = static_cast(tuple_slot) 
+ per_tuple_null_bitmap_bytes_;
  

[GitHub] incubator-quickstep pull request #100: Refactor bulk insert for SplitRowStor...

[navsan]

Github user navsan commented on a diff in the pull request:

https://github.com/apache/incubator-quickstep/pull/100#discussion_r79695934
  
--- Diff: storage/SplitRowStoreTupleStorageSubBlock.cpp ---
@@ -88,6 +88,67 @@ inline std::size_t 
CalculateVariableSizeWithRemappedAttributes(
   return total_size;
 }
 
+
+/**
+ * A struct which holds the offset information for a non-remapping insert
+ * operation
+ */
+struct BasicInsertInfo {
+  BasicInsertInfo(
+const CatalogRelationSchema &relation)
+: num_attrs_(relation.size()),
+  num_nullable_attrs_(relation.numNullableAttributes()),
+  max_var_length_(relation.getMaximumVariableByteLength()),
+  fixed_len_offset_(BitVector::BytesNeeded(num_nullable_attrs_)),
+  var_len_offset_(fixed_len_offset_ + relation.getFixedByteLength()),
+  is_variable_(num_attrs_),
+  is_nullable_(num_attrs_),
+  fixed_len_offsets_(num_attrs_),
+  fixed_len_sizes_(num_attrs_),
+  var_len_offsets_(num_attrs_) {
+attribute_id accessor_attr_id = 0;
+for (CatalogRelationSchema::const_iterator attr_it = relation.begin();
+ attr_it != relation.end();
+ ++attr_it, ++accessor_attr_id) {
+  DCHECK_EQ(accessor_attr_id, attr_it->getID());
+
+  const int nullable_idx = 
relation.getNullableAttributeIndex(accessor_attr_id);
+  const int variable_idx = 
relation.getVariableLengthAttributeIndex(accessor_attr_id);
+  is_nullable_.setBit(accessor_attr_id, nullable_idx != -1);
+
+  if (variable_idx == -1) {
+is_variable_.setBit(accessor_attr_id, false);
+fixed_len_offsets_[accessor_attr_id] = 
relation.getFixedLengthAttributeOffset(accessor_attr_id);
+fixed_len_sizes_[accessor_attr_id] = relation.getAttributeById(
+  accessor_attr_id)->getType().maximumByteLength();
+var_len_offsets_[accessor_attr_id] = -1;
+  } else {
+is_variable_.setBit(accessor_attr_id, true);
+fixed_len_offsets_[accessor_attr_id] = 0;
+fixed_len_sizes_[accessor_attr_id] = 0;
+var_len_offsets_[accessor_attr_id] = 
relation.getVariableLengthAttributeIndex(accessor_attr_id);
+  }
+}
+  }
+
+  std::size_t num_attrs_;
+  std::size_t num_nullable_attrs_;
+  std::size_t max_var_length_;
+
+  // byte offset from the beginning of a tuple to the first fixed length 
attribute
+  std::uint32_t fixed_len_offset_;
+  // byte offset from the beginning of a tuple to the first variable 
length offset/length pair
+  std::uint32_t var_len_offset_;
+
+  BitVector is_variable_;
--- End diff --

Actually, I think it's even better to just create one struct for each 
column, with all the info we care about for that column, like whether it's 
nullable/variable length and what its size is. That way, InsertInfo will have a 
vector which is better than a bunch of vectors you have (because 
the struct packed with all the info you need can just be unpacked into 
registers directly, whereas the vector will force you to do memory loads for 
each column for each tuple insertion.

See my bulkInsert function in PackedRowStore.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---