You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/12/03 20:27:57 UTC
[GitHub] [arrow] supunkamburugamuve opened a new pull request #11854: ARROW-12629: [C++] Option for disabling read ahead for csv and json readeers
supunkamburugamuve opened a new pull request #11854:
URL: https://github.com/apache/arrow/pull/11854
ARROW-12090 - Adding use_read_ahead parameter to json/options.h and csv/options.h. This option is used by the csv SerialReader and json Table reader to control the read ahead thread creation.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] supunkamburugamuve commented on a change in pull request #11854: ARROW-12629: [C++] Add option for disabling readahead in CSV and JSON readers
Posted by GitBox <gi...@apache.org>.
supunkamburugamuve commented on a change in pull request #11854:
URL: https://github.com/apache/arrow/pull/11854#discussion_r768798972
##########
File path: cpp/src/arrow/json/options.h
##########
@@ -65,6 +65,8 @@ struct ARROW_EXPORT ReadOptions {
/// Block size we request from the IO layer; also determines the size of
/// chunks when use_threads is true
int32_t block_size = 1 << 20; // 1 MB
+ /// weather we will use a separate thread for read ahead
+ bool use_read_ahead = true;
Review comment:
Done
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] supunkamburugamuve commented on a change in pull request #11854: ARROW-12629: [C++] Add option for disabling readahead in CSV and JSON readers
Posted by GitBox <gi...@apache.org>.
supunkamburugamuve commented on a change in pull request #11854:
URL: https://github.com/apache/arrow/pull/11854#discussion_r768798590
##########
File path: cpp/src/arrow/json/options.h
##########
@@ -65,6 +65,8 @@ struct ARROW_EXPORT ReadOptions {
/// Block size we request from the IO layer; also determines the size of
/// chunks when use_threads is true
int32_t block_size = 1 << 20; // 1 MB
+ /// weather we will use a separate thread for read ahead
Review comment:
Done
##########
File path: cpp/src/arrow/csv/options.h
##########
@@ -166,6 +166,9 @@ struct ARROW_EXPORT ReadOptions {
/// Create read options with default values
static ReadOptions Defaults();
+ /// weather to use an IO thread to read ahead
Review comment:
Done
##########
File path: cpp/src/arrow/csv/options.h
##########
@@ -166,6 +166,9 @@ struct ARROW_EXPORT ReadOptions {
/// Create read options with default values
static ReadOptions Defaults();
+ /// weather to use an IO thread to read ahead
+ bool use_read_ahead = false;
Review comment:
Done
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #11854: ARROW-12629: [C++] Option for disabling read ahead for csv and json readeers
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #11854:
URL: https://github.com/apache/arrow/pull/11854#discussion_r767959155
##########
File path: cpp/src/arrow/csv/options.h
##########
@@ -166,6 +166,9 @@ struct ARROW_EXPORT ReadOptions {
/// Create read options with default values
static ReadOptions Defaults();
+ /// weather to use an IO thread to read ahead
Review comment:
```suggestion
/// Whether to use an IO thread to read ahead while other blocks are decoding
```
##########
File path: cpp/src/arrow/json/options.h
##########
@@ -65,6 +65,8 @@ struct ARROW_EXPORT ReadOptions {
/// Block size we request from the IO layer; also determines the size of
/// chunks when use_threads is true
int32_t block_size = 1 << 20; // 1 MB
+ /// weather we will use a separate thread for read ahead
+ bool use_read_ahead = true;
Review comment:
```suggestion
bool use_readahead = true;
```
##########
File path: cpp/src/arrow/csv/options.h
##########
@@ -166,6 +166,9 @@ struct ARROW_EXPORT ReadOptions {
/// Create read options with default values
static ReadOptions Defaults();
+ /// weather to use an IO thread to read ahead
Review comment:
(feel free to reword differently if you find a better wording)
##########
File path: cpp/src/arrow/csv/options.h
##########
@@ -166,6 +166,9 @@ struct ARROW_EXPORT ReadOptions {
/// Create read options with default values
static ReadOptions Defaults();
+ /// weather to use an IO thread to read ahead
+ bool use_read_ahead = false;
Review comment:
Also, probably name this `use_readahead` ("readahead" is a noun in this context)
##########
File path: cpp/src/arrow/csv/options.h
##########
@@ -166,6 +166,9 @@ struct ARROW_EXPORT ReadOptions {
/// Create read options with default values
static ReadOptions Defaults();
+ /// weather to use an IO thread to read ahead
+ bool use_read_ahead = false;
Review comment:
Is there any reason to make this false by default?
##########
File path: cpp/src/arrow/json/options.h
##########
@@ -65,6 +65,8 @@ struct ARROW_EXPORT ReadOptions {
/// Block size we request from the IO layer; also determines the size of
/// chunks when use_threads is true
int32_t block_size = 1 << 20; // 1 MB
+ /// weather we will use a separate thread for read ahead
Review comment:
Can you update the comment to match the one in the CSV options?
##########
File path: cpp/src/arrow/json/reader_test.cc
##########
@@ -35,10 +35,11 @@ using util::string_view;
using internal::checked_cast;
-class ReaderTest : public ::testing::TestWithParam<bool> {
+class ReaderTest : public ::testing::TestWithParam<std::pair<bool, bool>> {
Review comment:
This will start being a bit cryptic. Perhaps:
```suggestion
struct ReaderTestParam {
bool use_threads;
bool use_readahead;
};
class ReaderTest : public ::testing::TestWithParam<ReaderTestParam> {
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #11854: ARROW-12629: [C++] Option for disabling read ahead for csv and json readeers
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11854:
URL: https://github.com/apache/arrow/pull/11854#issuecomment-985806897
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] supunkamburugamuve commented on a change in pull request #11854: ARROW-12629: [C++] Add option for disabling readahead in CSV and JSON readers
Posted by GitBox <gi...@apache.org>.
supunkamburugamuve commented on a change in pull request #11854:
URL: https://github.com/apache/arrow/pull/11854#discussion_r768876962
##########
File path: cpp/src/arrow/json/reader_test.cc
##########
@@ -35,10 +35,11 @@ using util::string_view;
using internal::checked_cast;
-class ReaderTest : public ::testing::TestWithParam<bool> {
+class ReaderTest : public ::testing::TestWithParam<std::pair<bool, bool>> {
Review comment:
Done
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org