| Message ID | 83fa4a5f3a5c79fa814932c0705867ff16a584c7.1648616734.git.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4d33e2ba6b9db6a28085b2ef7e46d30a981875ab |
| Headers | show |
| Series | core.fsyncmethod: add 'batch' mode for faster fsyncing of multiple objects | expand |
"Neeraj Singh via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Neeraj Singh <neerajsi@microsoft.com> > > Take advantage of the odb transaction infrastructure around writing the > cached tree to the object database. > > Signed-off-by: Neeraj Singh <neerajsi@microsoft.com> > --- > cache-tree.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/cache-tree.c b/cache-tree.c > index 6752f69d515..8c5e8822716 100644 > --- a/cache-tree.c > +++ b/cache-tree.c > @@ -3,6 +3,7 @@ > #include "tree.h" > #include "tree-walk.h" > #include "cache-tree.h" > +#include "bulk-checkin.h" > #include "object-store.h" > #include "replace-object.h" > #include "promisor-remote.h" > @@ -474,8 +475,10 @@ int cache_tree_update(struct index_state *istate, int flags) > > trace_performance_enter(); > trace2_region_enter("cache_tree", "update", the_repository); There is no I/O in update_one() when the WRITE_TREE_DRY_RUN bit is set, so we _could_ optimize the begin/end away with if (!(flags & WRITE_TREE_DRY_RUN)) begin_odb_transaction() > + begin_odb_transaction(); > i = update_one(istate->cache_tree, istate->cache, istate->cache_nr, > "", 0, &skip, flags); > + end_odb_transaction(); > trace2_region_leave("cache_tree", "update", the_repository); > trace_performance_leave("cache_tree_update"); > if (i < 0) I do not know if that is worth it. If we do not do any object creation inside begin/end, we don't even create the temporary object directory and there is nothing we need to do when we "unplug". So this would be fine as-is, but I may be overlooking something, so I thought I'd mention it for completeness. Thanks.
On Wed, Mar 30, 2022 at 10:47 AM Junio C Hamano <gitster@pobox.com> wrote: > > "Neeraj Singh via GitGitGadget" <gitgitgadget@gmail.com> writes: > > > From: Neeraj Singh <neerajsi@microsoft.com> > > > > Take advantage of the odb transaction infrastructure around writing the > > cached tree to the object database. > > > > Signed-off-by: Neeraj Singh <neerajsi@microsoft.com> > > --- > > cache-tree.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/cache-tree.c b/cache-tree.c > > index 6752f69d515..8c5e8822716 100644 > > --- a/cache-tree.c > > +++ b/cache-tree.c > > @@ -3,6 +3,7 @@ > > #include "tree.h" > > #include "tree-walk.h" > > #include "cache-tree.h" > > +#include "bulk-checkin.h" > > #include "object-store.h" > > #include "replace-object.h" > > #include "promisor-remote.h" > > @@ -474,8 +475,10 @@ int cache_tree_update(struct index_state *istate, int flags) > > > > trace_performance_enter(); > > trace2_region_enter("cache_tree", "update", the_repository); > > There is no I/O in update_one() when the WRITE_TREE_DRY_RUN bit is > set, so we _could_ optimize the begin/end away with > > if (!(flags & WRITE_TREE_DRY_RUN)) > begin_odb_transaction() > > > + begin_odb_transaction(); > > i = update_one(istate->cache_tree, istate->cache, istate->cache_nr, > > "", 0, &skip, flags); > > + end_odb_transaction(); > > trace2_region_leave("cache_tree", "update", the_repository); > > trace_performance_leave("cache_tree_update"); > > if (i < 0) > > I do not know if that is worth it. If we do not do any object > creation inside begin/end, we don't even create the temporary object > directory and there is nothing we need to do when we "unplug". So > this would be fine as-is, but I may be overlooking something, so I > thought I'd mention it for completeness. > Yes, with the current series, beginning and ending a transaction will just manipulate a few global variables in bulk-checkin.c unless there is something real to flush. Thanks, Neeraj
diff --git a/cache-tree.c b/cache-tree.c index 6752f69d515..8c5e8822716 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -3,6 +3,7 @@ #include "tree.h" #include "tree-walk.h" #include "cache-tree.h" +#include "bulk-checkin.h" #include "object-store.h" #include "replace-object.h" #include "promisor-remote.h" @@ -474,8 +475,10 @@ int cache_tree_update(struct index_state *istate, int flags) trace_performance_enter(); trace2_region_enter("cache_tree", "update", the_repository); + begin_odb_transaction(); i = update_one(istate->cache_tree, istate->cache, istate->cache_nr, "", 0, &skip, flags); + end_odb_transaction(); trace2_region_leave("cache_tree", "update", the_repository); trace_performance_leave("cache_tree_update"); if (i < 0)