DfsObjectDatabase: Expose PackList and move markDirty there

What's invalidated when an object database is "dirty" is not the whole
database, but rather a specific list of packs. If there is a race
between getting the pack list and setting the volatile dirty flag
where the packs are rescanned, we don't need to mark the new pack list
as dirty.

This is a fine point that only really applies if the decision of
whether or not to mark dirty actually requires introspecting the pack
list (say, its timestamps). The general operation of "take whatever
is the current pack list and mark it dirty" may still be inherently
racy, but the cost is not so high.

Change-Id: I159e9154bd8b2d348b4e383627a503e85462dcc6
This commit is contained in:
Dave Borowitz 2016-07-14 12:11:51 -04:00
parent 18e9db306b
commit 0f1c361e62
2 changed files with 37 additions and 20 deletions

View File

@ -68,7 +68,7 @@ boolean dirty() {
}
@Override
void markDirty() {
public void markDirty() {
// Always dirty.
}
};
@ -186,7 +186,16 @@ public DfsPackFile[] getPacks() throws IOException {
return getPackList().packs;
}
PackList getPackList() throws IOException {
/**
* Scan and list all available pack files in the repository.
*
* @return list of available packs, with some additional metadata. The
* returned array is shared with the implementation and must not be
* modified by the caller.
* @throws IOException
* the pack list cannot be initialized.
*/
public PackList getPackList() throws IOException {
return scanPacks(NO_PACKS);
}
@ -202,7 +211,18 @@ protected DfsRepository getRepository() {
* implementation and must not be modified by the caller.
*/
public DfsPackFile[] getCurrentPacks() {
return packList.get().packs;
return getCurrentPackList().packs;
}
/**
* List currently known pack files in the repository, without scanning.
*
* @return list of available packs, with some additional metadata. The
* returned array is shared with the implementation and must not be
* modified by the caller.
*/
public PackList getCurrentPackList() {
return packList.get();
}
/**
@ -460,17 +480,6 @@ protected void clearCache() {
packList.set(NO_PACKS);
}
/**
* Mark object database as dirty.
* <p>
* Used when the caller knows that new data might have been written to the
* repository that could invalidate open readers, for example if refs are
* newly scanned.
*/
protected void markDirty() {
packList.get().markDirty();
}
@Override
public void close() {
// PackList packs = packList.get();
@ -481,17 +490,25 @@ public void close() {
// p.close();
}
static abstract class PackList {
/** Snapshot of packs scanned in a single pass. */
public static abstract class PackList {
/** All known packs, sorted. */
final DfsPackFile[] packs;
public final DfsPackFile[] packs;
PackList(final DfsPackFile[] packs) {
PackList(DfsPackFile[] packs) {
this.packs = packs;
}
abstract boolean dirty();
abstract void markDirty();
/**
* Mark pack list as dirty.
* <p>
* Used when the caller knows that new data might have been written to the
* repository that could invalidate open readers depending on this pack list,
* for example if refs are newly scanned.
*/
public abstract void markDirty();
}
private static final class PackListImpl extends PackList {
@ -507,7 +524,7 @@ boolean dirty() {
}
@Override
void markDirty() {
public void markDirty() {
dirty = true;
}
}

View File

@ -310,7 +310,7 @@ protected RefCache scanAllRefs() throws IOException {
}
ids.sort();
sym.sort();
objdb.markDirty();
objdb.getCurrentPackList().markDirty();
return new RefCache(ids.toRefList(), sym.toRefList());
}