From b0f333b0340c0b34c014519d9b26901766b2053a Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Wed, 23 Mar 2016 16:39:16 -0700 Subject: [PATCH 1/3] Cache repository sync return values Affects: git_blob_owner git_commit_owner git_filter_source_repo git_index_owner git_object_owner git_reference_owner git_remote_owner git_revwalk_repository git_submodule_owner git_tag_owner git_tree_owner --- generate/input/descriptor.json | 1 + generate/scripts/helpers.js | 1 + generate/templates/partials/sync_function.cc | 41 ++++++++++++++++++- generate/templates/templates/class_content.cc | 10 +++++ generate/templates/templates/class_header.h | 8 ++++ 5 files changed, 60 insertions(+), 1 deletion(-) diff --git a/generate/input/descriptor.json b/generate/input/descriptor.json index 29f9e2ce4..46503da33 100644 --- a/generate/input/descriptor.json +++ b/generate/input/descriptor.json @@ -1767,6 +1767,7 @@ "ignore": true }, "repository": { + "cacheResult": true, "functions": { "git_repository__cleanup": { "ignore": true diff --git a/generate/scripts/helpers.js b/generate/scripts/helpers.js index 6e1f33613..6f3341209 100644 --- a/generate/scripts/helpers.js +++ b/generate/scripts/helpers.js @@ -179,6 +179,7 @@ var Helpers = { typeDef.filename = typeDef.typeName; typeDef.isLibgitType = true; typeDef.dependencies = []; + typeDef.cacheResult = Boolean(typeDefOverrides.cacheResult); typeDef.selfFreeing = Boolean(typeDefOverrides.selfFreeing); if (typeDefOverrides.freeFunctionName) { diff --git a/generate/templates/partials/sync_function.cc b/generate/templates/partials/sync_function.cc index ada0de62f..976061e3d 100644 --- a/generate/templates/partials/sync_function.cc +++ b/generate/templates/partials/sync_function.cc @@ -2,6 +2,11 @@ {%partial doc .%} NAN_METHOD({{ cppClassName }}::{{ cppFunctionName }}) { Nan::EscapableHandleScope scope; + +{%if return.cacheResult %} + {{ cppClassName }} *thisObj = Nan::ObjectWrap::Unwrap<{{ cppClassName }}>(info.This()); + return info.GetReturnValue().Set(scope.Escape(Nan::New(thisObj->{{ cppFunctionName }}_cachedResult))); +{% else %} {%partial guardArguments .%} {%each .|returnsInfo 'true' as _return %} @@ -35,7 +40,7 @@ if (Nan::ObjectWrap::Unwrap<{{ cppClassName }}>(info.This())->GetValue() != NULL {% endif %} giterr_clear(); - + { LockMaster lockMaster(true{%each args|argsInfo as arg %} {%if arg.cType|isPointer%}{%if not arg.isReturn%} @@ -125,4 +130,38 @@ if (Nan::ObjectWrap::Unwrap<{{ cppClassName }}>(info.This())->GetValue() != NULL {%endif%} {%endif%} } +{%endif%} } + +{%if return.cacheResult %} +void {{ cppClassName }}::{{ cppFunctionName }}_cache() { + if (!raw) { + {{ cppFunctionName }}_cachedResult.Reset(Nan::Null()); + return; + } + + LockMaster lockMaster(true{%each args|argsInfo as arg %} + {%if arg.cType|isPointer%}{%if not arg.isReturn%} + ,{%if arg.isSelf %} + raw + {%endif%} + {%endif%}{%endif%} + {%endeach%}); + + {{ return.cType }} result = {{ cFunctionName }}( + {%each args|argsInfo as arg %} + {%if arg.isSelf %} + raw + {%endif%} + {%endeach%} + ); + + Local to; + + {%each .|returnsInfo as _return %} + {%partial convertToV8 _return %} + {%endeach%} + + {{ cppFunctionName }}_cachedResult.Reset(to); +} +{% endif %} diff --git a/generate/templates/templates/class_content.cc b/generate/templates/templates/class_content.cc index 312c2ef28..993b11663 100644 --- a/generate/templates/templates/class_content.cc +++ b/generate/templates/templates/class_content.cc @@ -44,6 +44,16 @@ using namespace node; NonSelfFreeingConstructedCount++; } + + {%each functions as function%} + {%if not function.ignore %} + {%if not function.isAsync %} + {%if function.return.cacheResult %} + {{ function.cppFunctionName }}_cache(); // populate cached value + {%endif%} + {%endif%} + {%endif%} + {%endeach%} } {{ cppClassName }}::~{{ cppClassName }}() { diff --git a/generate/templates/templates/class_header.h b/generate/templates/templates/class_header.h index cc77f52b3..9e09ad731 100644 --- a/generate/templates/templates/class_header.h +++ b/generate/templates/templates/class_header.h @@ -141,6 +141,14 @@ class {{ cppClassName }} : public Nan::ObjectWrap { private: {{ function.cppFunctionName }}Baton *baton; }; + {%else%} + {%if function.return.cacheResult %} + // For simple sync functions that return a wrapped object and pass `raw` + // as the the only parameter to libgit2, we cache the results. + // CopyablePersistentTraits are used to get the reset-on-destruct behavior. + void {{ function.cppFunctionName }}_cache(); + Nan::Persistent > {{ function.cppFunctionName }}_cachedResult; + {%endif%} {%endif%} static NAN_METHOD({{ function.cppFunctionName }}); From c23d58fd62a1429d35bfcc1afdc9cf32879853d2 Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Tue, 22 Mar 2016 17:18:02 -0700 Subject: [PATCH 2/3] Add test for owner caching --- test/tests/commit.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/tests/commit.js b/test/tests/commit.js index e770a0c1e..c457dc01d 100644 --- a/test/tests/commit.js +++ b/test/tests/commit.js @@ -404,6 +404,12 @@ describe("Commit", function() { assert.ok(owner instanceof Repository); }); + it("caches its owner", function() { + var owner = this.commit.owner(); + var ownerAgain = this.commit.owner(); + assert.ok(owner === ownerAgain); + }); + it("can walk its repository's history", function(done) { var historyCount = 0; var expectedHistoryCount = 364; From cd21a0f3421eea03821b14c18d2afa27ecf21350 Mon Sep 17 00:00:00 2001 From: Stjepan Rajko Date: Thu, 24 Mar 2016 00:03:33 -0700 Subject: [PATCH 3/3] Pass id instead of object to createBranch --- test/tests/submodule.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/tests/submodule.js b/test/tests/submodule.js index 002b4ef12..d4f9a9f70 100644 --- a/test/tests/submodule.js +++ b/test/tests/submodule.js @@ -134,7 +134,7 @@ describe("Submodule", function() { return reference.peel(NodeGit.Object.TYPE.COMMIT); }) .then(function(commit) { - return submoduleRepo.createBranch("master", commit); + return submoduleRepo.createBranch("master", commit.id()); }) .then(function() { return submodule.addFinalize();