Re: [EGIT PATCH 3/6] Add a method to get refs by object Id

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Robin Rosenberg <robin.rosenberg@xxxxxxxxxx> wrote:
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> index dfce1b8..3fc5236 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
> @@ -939,6 +940,33 @@ public String getBranch() throws IOException {
>  	}
>  
>  	/**
> +	 * @return a map with all objects referenced by a peeled ref.
> +	 */
> +	public Map<AnyObjectId, List<Ref>> getAllRefsByPeeledObjectId() {

Do we really want to promise List here?  Can we make it just
Collection instead?

> +		Map<String, Ref> allRefs = getAllRefs();
> +		HashMap<AnyObjectId, List<Ref>> ret = new HashMap<AnyObjectId, List<Ref>>(allRefs.size());
> +		for (Map.Entry<String,Ref> e : allRefs.entrySet()) {
> +			Ref ref = e.getValue();

I think this is cleaner:

	for (Ref ref : allRefs.values()) {

as you never use the key.

> +			AnyObjectId target = ref.getPeeledObjectId();
> +			if (target == null)
> +				target = ref.getObjectId();
> +			List<Ref> list = ret.get(target);
> +			if (list == null) {
> +				list = Collections.singletonList(ref);
> +			} else {
> +				if (list.size() == 1) {
> +					ArrayList<Ref> list2 = new ArrayList<Ref>(2);
> +					list2.add(list.get(0));
> +					list = list2;
> +				}
> +				list.add(ref);
> +			}
> +			ret.put(target, list);

Hmm.  Putting the list every time is pointless.  This is may run
faster because we (on average) only do one hash lookup per target,
not 2:

	List<Ref> nl = Collections.singletonList(ref);
	List<Ref> ol = ret.put(target, nl);
	if (ol != null) {
		if (ol.size() == 1) {
			nl = new ArrayList<Ref>(2);
			nl.add(ol.get(0));
			nl.add(ref);
			ret.put(target, nl);
		} else {
			ol.add(ref)
			ret.put(target, ol);
		}
	}

The trick is that most targets have exactly one Ref, so the first
ret.put will return null and we'll never worry about the expansion
cases.  In the rare cases where more than one Ref points at the
same object we'll degrade into doing two hash lookups per target,
which is no worse than what you have right now.

-- 
Shawn.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux