-
Notifications
You must be signed in to change notification settings - Fork 13
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes for the merge operations #26
base: develop
Are you sure you want to change the base?
Changes for the merge operations #26
Conversation
changes in the vol_async file |
src/h5_async_vol.c
Outdated
@@ -1,5 +1,5 @@ | |||
/******************************************************************************* | |||
Asynchronous I/O VOL Connector (AsyncVOL) Copyright (c) 2021, The | |||
Asynchronous I/O VOL kamal Connector (AsyncVOL) Copyright (c) 2021, The |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reverse this change / remove your name
src/h5_async_vol.c
Outdated
@@ -65,7 +65,7 @@ works, and perform publicly and display publicly, and to permit others to do so. | |||
/* Whether to display log messge when callback is invoked */ | |||
/* (Uncomment to enable) */ | |||
/* #define ENABLE_DBG_MSG 1 */ | |||
/* #define PRINT_ERROR_STACK 1 */ | |||
#define PRINT_ERROR_STACK 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reverse this change
src/h5_async_vol.c
Outdated
bool disable_implicit_file; /* Disable implicit async execution globally */ | ||
bool disable_implicit; /* Disable implicit async execution for dxpl */ | ||
bool delay_time_env; /* Flag that indicates the delay time is set by env variable */ | ||
bool disable_async_dset_get; /* Disable async execution for dataset get */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add disable_async_dset_get line back
src/h5_async_vol.c
Outdated
@@ -256,8 +256,9 @@ typedef struct async_attr_write_args_t { | |||
void * buf; | |||
hid_t dxpl_id; | |||
void **req; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove empty line
src/h5_async_vol.c
Outdated
@@ -331,8 +332,9 @@ typedef struct async_dataset_write_args_t { | |||
hid_t plist_id; | |||
void * buf; | |||
void **req; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove empty line
test/2d_asyn_MPI_write.c
Outdated
\n",offset_out[0],offset_out[1],count_out[0],count_out[1]); | ||
|
||
status = H5Dwrite_async (dataset, H5T_NATIVE_INT, memspace, dataspace, data_transfer_propertylist, | ||
data,es_id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these commented out code useful? If not, remove them.
test/3d_asyn_MPI_write.c
Outdated
{1,1,4,2,2,2} | ||
} | ||
|
||
}; */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these commented out code useful? If not, remove them.
test/3d_asyn_MPI_write.c
Outdated
data,es_id); | ||
|
||
|
||
} */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these commented out code useful? If not, remove them.
test/Makefile
Outdated
@@ -37,7 +37,8 @@ SRC = async_test_serial.c async_test_serial2.c \ | |||
async_test_parallel.c async_test_parallel2.c async_test_parallel3.c \ | |||
async_test_parallel4.c async_test_parallel5.c \ | |||
async_test_serial_event_set.c async_test_serial_error_stack.c \ | |||
async_test_serial_event_set_error_stack.c | |||
async_test_serial_event_set_error_stack.c 1d_asyn_MPI_write.c 2d_asyn_MPI_write.c \ | |||
3d_asyn_MPI_write.c 1d_async_write_performance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1d_async_write_performance -> 1d_async_write_performance.c
src/h5_async_vol.c
Outdated
|
||
if (return_val == 1) | ||
goto done; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add #ifdef ENABLE_WRITE_MERGE ... #endif before 9973 and after 9977, so that the merge only happens when enabled at compile time
src/h5_async_vol.c
Outdated
@@ -1,5 +1,5 @@ | |||
/******************************************************************************* | |||
Asynchronous I/O VOL Connector (AsyncVOL) Copyright (c) 2021, The | |||
Asynchronous I/O VOL Connector (AsyncVOL) Copyright (c) 2021, The |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the extra space
src/h5_async_vol.c
Outdated
@@ -65,7 +65,7 @@ works, and perform publicly and display publicly, and to permit others to do so. | |||
/* Whether to display log messge when callback is invoked */ | |||
/* (Uncomment to enable) */ | |||
/* #define ENABLE_DBG_MSG 1 */ | |||
/* #define PRINT_ERROR_STACK 1 */ | |||
#define PRINT_ERROR_STACK 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
revert to commented line
@@ -15,7 +15,7 @@ ABT_LIBDIR = $(shell if [ -d $$ABT_DIR/lib ]; then echo "lib"; else echo "lib64 | |||
|
|||
# These lines may need to be adjusted for your compiler environment: | |||
DEBUG = -g -O0 | |||
CFLAGS = $(DEBUG) -fPIC -I$(ABT_DIR)/include -I$(HDF5_DIR)/include -Wall | |||
CFLAGS = $(DEBUG) -fPIC -I$(ABT_DIR)/include -I$(HDF5_DIR)/include -Wall -DENABLE_WRITE_MERGE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove "-DENABLE_WRITE_MERGE", it should not be enabled by default
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kamalchowdhurylbl Please rerun the clang-format tool with "clang-format -i -style=file ./*.c", there are too many formatting changes with your latest commit.
@@ -15,7 +15,7 @@ ABT_LIBDIR = $(shell if [ -d $$ABT_DIR/lib ]; then echo "lib"; else echo "lib64 | |||
|
|||
# These lines may need to be adjusted for your compiler environment: | |||
DEBUG = -g -O0 | |||
CFLAGS = $(DEBUG) -fPIC -I$(ABT_DIR)/include -I$(HDF5_DIR)/include -Wall | |||
CFLAGS = $(DEBUG) -fPIC -I$(ABT_DIR)/include -I$(HDF5_DIR)/include -Wall -DENABLE_WRITE_MERGE |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-DENABLE_WRITE_MERGE should be removed/commented out to not turn on the merge feature by default, until we do more testing
@@ -105,7 +105,7 @@ FILE *fout_g; | |||
/* The async VOL wrapper context */ | |||
typedef struct H5VL_async_wrap_ctx_t { | |||
hid_t under_vol_id; /* VOL ID for under VOL */ | |||
void * under_wrap_ctx; /* Object wrapping context for under VOL */ | |||
void *under_wrap_ctx; /* Object wrapping context for under VOL */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to use the https://github.com/hpc-io/vol-async/blob/develop/.clang-format file when you run clang-format command, e.g. under vol-async/src do "clang-format -i -style=file ./*.c"
@houjun , do you know the status of this PR? Was it just a matter of format changes? Should we sync it with develop and format it? |
@brtnfld, this is not ready to merge yet, I'll change it to a draft. |
No description provided.