pitrou commented on code in PR #14225:
URL: https://github.com/apache/arrow/pull/14225#discussion_r980974105
##########
cpp/src/arrow/memory_pool.h:
##########
@@ -65,6 +65,8 @@ class MemoryPoolStats {
/// take care of the required 64-byte alignment.
class ARROW_EXPORT MemoryPool {
public:
+ static constexpr int64_t kDefaultAlignment = 64;
Review Comment:
It's a bit of a pity to have to include `memory_pool.h` just for this, can
we instead make it a global constant (e.g. `kDefaultMemoryAlignment`) and move
it to `type_fwd.h`?
##########
cpp/src/arrow/testing/random.h:
##########
@@ -49,7 +50,9 @@ class ARROW_TESTING_EXPORT RandomArrayGenerator {
/// \param[in] null_probability the probability of a bit being zero
///
/// \return a generated Buffer
- std::shared_ptr<Buffer> NullBitmap(int64_t size, double null_probability =
0);
+ std::shared_ptr<Buffer> NullBitmap(int64_t size, double null_probability = 0,
+ int64_t alignment =
MemoryPool::kDefaultAlignment,
+ MemoryPool* memory_pool = NULLPTR);
Review Comment:
These methods are purely to generate testing data, do we really care about
alignment here?
##########
cpp/src/arrow/buffer_builder.h:
##########
@@ -198,6 +203,7 @@ class ARROW_EXPORT BufferBuilder {
uint8_t* data_;
int64_t capacity_;
int64_t size_;
+ int64_t alignment_;
Review Comment:
```suggestion
const int64_t alignment_;
```
##########
cpp/src/arrow/compute/exec/test_util.h:
##########
@@ -106,7 +106,9 @@ BatchesWithSchema MakeNestedBatches();
ARROW_TESTING_EXPORT
BatchesWithSchema MakeRandomBatches(const std::shared_ptr<Schema>& schema,
- int num_batches = 10, int batch_size = 4);
+ int num_batches = 10, int batch_size = 4,
+ int64_t alignment =
MemoryPool::kDefaultAlignment,
Review Comment:
Same question as for `testing/random.h`: is this useful?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]