-
Notifications
You must be signed in to change notification settings - Fork 16
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
Speedup render by avoiding repeated vector copies. #176
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
604a89a
Merge pull request #175 from mrc-ide/dev
giovannic 50a6d57
Update pkgdown.yml
giovannic ed06ad1
Fix logo on pkgdown
giovannic 066130b
Fix R build badge
giovannic 48e1a34
Speedup render by avoiding repeated vector copies.
plietar f0c21c5
Code review
plietar 17bc1d8
Switch the std::vector<double> instead of NumericVector.
plietar cf348b8
Update Rcpp to fix warnings.
plietar 250c388
Add test for error code path
plietar c9cdee3
Add benchmark for Render.
plietar File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
/* | ||
* RenderVector.h | ||
* | ||
* Created on: 21 Dec 2023 | ||
* Author: pl2113 | ||
*/ | ||
|
||
#ifndef INST_INCLUDE_RENDER_VECTOR_H_ | ||
#define INST_INCLUDE_RENDER_VECTOR_H_ | ||
|
||
#include <Rcpp.h> | ||
|
||
/** | ||
* A thin wrapper around a std::vector<double>, used to provide by-reference | ||
* semantics and guaranteed in-place mutation in the Render class. | ||
* | ||
*/ | ||
struct RenderVector { | ||
RenderVector(std::vector<double> data) : _data(std::move(data)) { } | ||
|
||
void update(size_t index, double value) { | ||
// index is R-style 1-indexed, rather than C's 0-indexing. | ||
if (index < 1 || index > _data.size()) { | ||
Rcpp::stop("index out-of-bounds"); | ||
} | ||
_data[index - 1] = value; | ||
} | ||
|
||
const std::vector<double>& data() const { | ||
return _data; | ||
} | ||
|
||
private: | ||
std::vector<double> _data; | ||
}; | ||
|
||
#endif /* INST_INCLUDE_RENDER_VECTOR_H_ */ |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -366,7 +366,7 @@ RcppExport SEXP _individual_dummy() { | |
if (rcpp_isError_gen) { | ||
SEXP rcpp_msgSEXP_gen = Rf_asChar(rcpp_result_gen); | ||
UNPROTECT(1); | ||
Rf_error(CHAR(rcpp_msgSEXP_gen)); | ||
Rf_error("%s", CHAR(rcpp_msgSEXP_gen)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was introduced by running against a prerelease of Rcpp to avoid a CI failure, as explained in RcppCore/Rcpp#1287 (comment). We only need the pre-release to generate this file. Users of the package don't need it. |
||
} | ||
UNPROTECT(1); | ||
return rcpp_result_gen; | ||
|
@@ -1303,6 +1303,40 @@ BEGIN_RCPP | |
return R_NilValue; | ||
END_RCPP | ||
} | ||
// create_render_vector | ||
Rcpp::XPtr<RenderVector> create_render_vector(std::vector<double> data); | ||
RcppExport SEXP _individual_create_render_vector(SEXP dataSEXP) { | ||
BEGIN_RCPP | ||
Rcpp::RObject rcpp_result_gen; | ||
Rcpp::RNGScope rcpp_rngScope_gen; | ||
Rcpp::traits::input_parameter< std::vector<double> >::type data(dataSEXP); | ||
rcpp_result_gen = Rcpp::wrap(create_render_vector(data)); | ||
return rcpp_result_gen; | ||
END_RCPP | ||
} | ||
// render_vector_update | ||
void render_vector_update(Rcpp::XPtr<RenderVector> v, size_t index, double value); | ||
RcppExport SEXP _individual_render_vector_update(SEXP vSEXP, SEXP indexSEXP, SEXP valueSEXP) { | ||
BEGIN_RCPP | ||
Rcpp::RNGScope rcpp_rngScope_gen; | ||
Rcpp::traits::input_parameter< Rcpp::XPtr<RenderVector> >::type v(vSEXP); | ||
Rcpp::traits::input_parameter< size_t >::type index(indexSEXP); | ||
Rcpp::traits::input_parameter< double >::type value(valueSEXP); | ||
render_vector_update(v, index, value); | ||
return R_NilValue; | ||
END_RCPP | ||
} | ||
// render_vector_data | ||
std::vector<double> render_vector_data(Rcpp::XPtr<RenderVector> v); | ||
RcppExport SEXP _individual_render_vector_data(SEXP vSEXP) { | ||
BEGIN_RCPP | ||
Rcpp::RObject rcpp_result_gen; | ||
Rcpp::RNGScope rcpp_rngScope_gen; | ||
Rcpp::traits::input_parameter< Rcpp::XPtr<RenderVector> >::type v(vSEXP); | ||
rcpp_result_gen = Rcpp::wrap(render_vector_data(v)); | ||
return rcpp_result_gen; | ||
END_RCPP | ||
} | ||
// execute_process | ||
void execute_process(Rcpp::XPtr<process_t> process, size_t timestep); | ||
RcppExport SEXP _individual_execute_process(SEXP processSEXP, SEXP timestepSEXP) { | ||
|
@@ -1475,6 +1509,9 @@ static const R_CallMethodDef CallEntries[] = { | |
{"_individual_integer_ragged_variable_queue_extend", (DL_FUNC) &_individual_integer_ragged_variable_queue_extend, 2}, | ||
{"_individual_integer_ragged_variable_queue_shrink", (DL_FUNC) &_individual_integer_ragged_variable_queue_shrink, 2}, | ||
{"_individual_integer_ragged_variable_queue_shrink_bitset", (DL_FUNC) &_individual_integer_ragged_variable_queue_shrink_bitset, 2}, | ||
{"_individual_create_render_vector", (DL_FUNC) &_individual_create_render_vector, 1}, | ||
{"_individual_render_vector_update", (DL_FUNC) &_individual_render_vector_update, 3}, | ||
{"_individual_render_vector_data", (DL_FUNC) &_individual_render_vector_data, 1}, | ||
{"_individual_execute_process", (DL_FUNC) &_individual_execute_process, 2}, | ||
{"_individual_variable_get_size", (DL_FUNC) &_individual_variable_get_size, 1}, | ||
{"_individual_variable_update", (DL_FUNC) &_individual_variable_update, 1}, | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
/* | ||
* render_vector.cpp | ||
* | ||
* Created on: 21 Dec 2023 | ||
* Author: pl2113 | ||
*/ | ||
|
||
|
||
#include "../inst/include/RenderVector.h" | ||
#include <Rcpp.h> | ||
|
||
|
||
//[[Rcpp::export]] | ||
Rcpp::XPtr<RenderVector> create_render_vector(std::vector<double> data) { | ||
return Rcpp::XPtr<RenderVector>(new RenderVector(std::move(data)), true); | ||
} | ||
|
||
//[[Rcpp::export]] | ||
void render_vector_update(Rcpp::XPtr<RenderVector> v, size_t index, double value) { | ||
v->update(index, value); | ||
} | ||
|
||
//[[Rcpp::export]] | ||
std::vector<double> render_vector_data(Rcpp::XPtr<RenderVector> v) { | ||
return v->data(); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
# | ||
# bench-render.R | ||
# | ||
# Created on: 22 Dec 2023 | ||
# Author: pl2113 | ||
# | ||
|
||
library(individual) | ||
library(bench) | ||
library(ggplot2) | ||
library(scales) | ||
|
||
source("./tests/performance/utils.R") | ||
|
||
render_single <- bench::press( | ||
timesteps=floor(10^seq(3,6,0.25)), | ||
{ | ||
render <- Render$new(timesteps) | ||
bench::mark( | ||
min_iterations = 50, | ||
check = FALSE, | ||
render={ | ||
# Use timesteps/2 to write in the middle of the array | ||
render$render("data", 0.5, timesteps/2) | ||
}) | ||
}) | ||
|
||
render_single %>% | ||
simplify_bench_output() %>% | ||
ggplot() + | ||
aes(x = timesteps, y = as.numeric(time), color=expression, fill=expression, group=as.factor(timesteps):expression) + | ||
geom_violin(position=position_dodge(width=0.02), alpha=0.3) + | ||
labs(y="time", fill="expression", color="expression") + | ||
scale_x_continuous(trans='log10', n.breaks=6, labels = label_comma()) + | ||
scale_y_continuous(trans='log10', n.breaks=6, labels = function(x) format(bench::as_bench_time(x))) + | ||
ggtitle("Render single timestep benchmark") | ||
|
||
render_all <- bench::press( | ||
timesteps=floor(10^seq(3,5,0.25)), | ||
{ | ||
data <- runif(timesteps) | ||
bench::mark( | ||
min_iterations = 5, | ||
check = FALSE, | ||
filter_gc = FALSE, | ||
render_all={ | ||
render <- Render$new(timesteps) | ||
mapply(function(x, i) render$render("data", x, i), data, seq_along(data)) | ||
}) | ||
}) | ||
|
||
render_all %>% | ||
simplify_bench_output(filter_gc=FALSE) %>% | ||
ggplot() + | ||
aes(x = timesteps, y = as.numeric(time), color=expression, fill=expression, group=as.factor(timesteps):expression) + | ||
geom_violin(position=position_dodge(width=0.01), alpha=0.3) + | ||
labs(y="time", fill="expression", color="expression") + | ||
scale_x_continuous(trans='log10', n.breaks=6, labels = label_comma()) + | ||
scale_y_continuous(trans='log10', n.breaks=6, labels = function(x) format(bench::as_bench_time(x))) + | ||
ggtitle("Render all timesteps benchmark") | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
This change comes from an Rcpp change at RcppCore/Rcpp#1256