Skip to content

Avoid unnecessary heap allocation of SLICE objects #542

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 25, 2016
Merged

Avoid unnecessary heap allocation of SLICE objects #542

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 22 additions & 39 deletions ext/nmatrix/ruby_nmatrix.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ static VALUE nm_each_stored_with_indices(VALUE nmatrix);
static VALUE nm_each_ordered_stored_with_indices(VALUE nmatrix);
static VALUE nm_map_stored(VALUE nmatrix);

static SLICE* get_slice(size_t dim, int argc, VALUE* arg, size_t* shape);
static void init_slice_no_alloc(SLICE* slice, size_t dim, int argc, VALUE* arg, size_t* shape);
static VALUE nm_xslice(int argc, VALUE* argv, void* (*slice_func)(const STORAGE*, SLICE*), void (*delete_func)(NMATRIX*), VALUE self);
static VALUE nm_mset(int argc, VALUE* argv, VALUE self);
static VALUE nm_mget(int argc, VALUE* argv, VALUE self);
Expand Down Expand Up @@ -398,28 +398,6 @@ void Init_nmatrix() {
// Ruby Methods //
//////////////////


/*
* Slice constructor.
*/
static SLICE* alloc_slice(size_t dim) {
SLICE* slice = NM_ALLOC(SLICE);
slice->coords = NM_ALLOC_N(size_t, dim);
slice->lengths = NM_ALLOC_N(size_t, dim);
return slice;
}


/*
* Slice destructor.
*/
static void free_slice(SLICE* slice) {
NM_FREE(slice->coords);
NM_FREE(slice->lengths);
NM_FREE(slice);
}


/*
* Allocator.
*/
Expand Down Expand Up @@ -1272,7 +1250,12 @@ static VALUE nm_init_new_version(int argc, VALUE* argv, VALUE self) {
tmp_shape[m] = shape[m];
}

SLICE* slice = get_slice(dim, dim, slice_argv, shape);
SLICE slice_s;
SLICE* slice = &slice_s;
slice->coords = NM_ALLOCA_N(size_t, dim);
slice->lengths = NM_ALLOCA_N(size_t, dim);
init_slice_no_alloc(slice, dim, dim, slice_argv, shape);

// Create a temporary dense matrix and use it to do a slice assignment on self.
NMATRIX* tmp = nm_create(nm::DENSE_STORE, (STORAGE*)nm_dense_storage_create(dtype, tmp_shape, dim, v, v_size));
nm_register_nmatrix(tmp);
Expand All @@ -1282,8 +1265,6 @@ static VALUE nm_init_new_version(int argc, VALUE* argv, VALUE self) {
if (stype == nm::YALE_STORE) nm_yale_storage_set(self, slice, rb_tmp);
else nm_list_storage_set(self, slice, rb_tmp);

free_slice(slice);

// We need to free v if it's not the same size as tmp -- because tmp will have made a copy instead.
//if (nm_storage_count_max_elements(tmp->storage) != v_size)
// NM_FREE(v);
Expand Down Expand Up @@ -2018,7 +1999,11 @@ static VALUE nm_mset(int argc, VALUE* argv, VALUE self) {
NM_CONSERVATIVE(nm_register_value(&self));
NM_CONSERVATIVE(nm_register_values(argv, argc));

SLICE* slice = get_slice(dim, argc-1, argv, NM_STORAGE(self)->shape);
SLICE slice_s;
SLICE* slice = &slice_s;
slice->coords = NM_ALLOCA_N(size_t, dim);
slice->lengths = NM_ALLOCA_N(size_t, dim);
init_slice_no_alloc(slice, dim, argc-1, argv, NM_STORAGE(self)->shape);

static void (*ttable[nm::NUM_STYPES])(VALUE, SLICE*, VALUE) = {
nm_dense_storage_set,
Expand All @@ -2028,8 +2013,6 @@ static VALUE nm_mset(int argc, VALUE* argv, VALUE self) {

ttable[NM_STYPE(self)](self, slice, argv[argc-1]);

free_slice(slice);

to_return = argv[argc-1];

NM_CONSERVATIVE(nm_unregister_value(&self));
Expand Down Expand Up @@ -2259,7 +2242,12 @@ static VALUE nm_xslice(int argc, VALUE* argv, void* (*slice_func)(const STORAGE*

nm_register_value(&result);

SLICE* slice = get_slice(NM_DIM(self), argc, argv, s->shape);
SLICE slice_s;
SLICE* slice = &slice_s;
size_t dim = NM_DIM(self);
slice->coords = NM_ALLOCA_N(size_t, dim);
slice->lengths = NM_ALLOCA_N(size_t, dim);
init_slice_no_alloc(slice, dim, argc, argv, s->shape);

if (slice->single) {
static void* (*ttable[nm::NUM_STYPES])(const STORAGE*, SLICE*) = {
Expand All @@ -2280,8 +2268,6 @@ static VALUE nm_xslice(int argc, VALUE* argv, void* (*slice_func)(const STORAGE*
result = Data_Wrap_Struct(CLASS_OF(self), nm_mark, delete_func, mat);
nm_unregister_nmatrix(mat);
}

free_slice(slice);
}

nm_unregister_value(&result);
Expand Down Expand Up @@ -2623,19 +2609,17 @@ nm::dtype_t nm_dtype_guess(VALUE v) {
}
}



/*
* Allocate and return a SLICE object, which will contain the appropriate coordinate and length information for
* accessing some part of a matrix.
* Modify an existing SLICE object (with properly allocated memory),
* so that it will contain the appropriate coordinate and length information
* for accessing some part of a matrix.
*/
static SLICE* get_slice(size_t dim, int argc, VALUE* arg, size_t* shape) {
static void init_slice_no_alloc(SLICE* slice, size_t dim, int argc, VALUE* arg, size_t* shape) {
NM_CONSERVATIVE(nm_register_values(arg, argc));

VALUE beg, end;
int excl;

SLICE* slice = alloc_slice(dim);
slice->single = true;

// r is the shape position; t is the slice position. They may differ when we're dealing with a
Expand Down Expand Up @@ -2693,7 +2677,6 @@ static SLICE* get_slice(size_t dim, int argc, VALUE* arg, size_t* shape) {
}

NM_CONSERVATIVE(nm_unregister_values(arg, argc));
return slice;
}

#ifdef BENCHMARK
Expand Down