Skip to content

perf(unionfind): optimize UnifyTermsExtend map duplication#91

Closed
apocalypse9949 wants to merge 3 commits intogoogle:mainfrom
apocalypse9949:unionfind
Closed

perf(unionfind): optimize UnifyTermsExtend map duplication#91
apocalypse9949 wants to merge 3 commits intogoogle:mainfrom
apocalypse9949:unionfind

Conversation

@apocalypse9949
Copy link
Contributor

Added a Copy() method to UnionFind that pre-allocates the map capacity using the length of the parent map (len(uf.parent)). This prevents unnecessary memory reallocations and rehashing when duplicating the union-find structure.

UnifyTermsExtend was refactored to use this Copy() method instead of creating an empty map and copying elements manually, resulting in a significant performance improvement for repetitive inference loops.

Before this change, copying unionfind base would iteratively populate an empty map:

// UnifyTermsExtend unifies two same-length lists of relational terms, returning
// an extended UnionFind. It does not handle apply-expressions.
func UnifyTermsExtend(xs []ast.BaseTerm, ys []ast.BaseTerm, base UnionFind) (UnionFind, error) {
	if len(xs) != len(ys) {
		return UnionFind{}, fmt.Errorf("not of equal size")
	}
	uf := UnionFind{make(map[ast.BaseTerm]ast.BaseTerm)}
	for k, v := range base.parent {
		uf.parent[k] = v
	}
// ...

After the change, memory pre-allocation avoids multiple resize pauses:

// Copy returns a new UnionFind that is a copy of this one.
func (uf UnionFind) Copy() UnionFind {
	newUf := UnionFind{make(map[ast.BaseTerm]ast.BaseTerm, len(uf.parent))}
	for k, v := range uf.parent {
		newUf.parent[k] = v
	}
	return newUf
}

// UnifyTermsExtend unifies two same-length lists of relational terms, returning
// an extended UnionFind. It does not handle apply-expressions.
func UnifyTermsExtend(xs []ast.BaseTerm, ys []ast.BaseTerm, base UnionFind) (UnionFind, error) {
	if len(xs) != len(ys) {
		return UnionFind{}, fmt.Errorf("not of equal size")
	}
	uf := base.Copy()
// ...

@google-cla
Copy link

google-cla bot commented Mar 14, 2026

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

burakemir added a commit that referenced this pull request Mar 15, 2026
Pre-allocates map capacity when copying the union-find structure,
avoiding unnecessary rehashing during term unification.

Based on github.com//pull/91

Co-Authored-By: apocalypse9949 <dchittibabu641@gmail.com>
@burakemir
Copy link
Collaborator

Added in 69dc00f - thanks for your contribution!

@burakemir burakemir closed this Mar 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants