Skip to content

Make index.addAll use status to increase performance - #475

Merged
johnhaley81 merged 5 commits into
masterfrom
speedup-index-addall
Mar 10, 2015
Merged

Make index.addAll use status to increase performance#475
johnhaley81 merged 5 commits into
masterfrom
speedup-index-addall

Conversation

@maxkorp

@maxkorp maxkorp commented Mar 9, 2015

Copy link
Copy Markdown
Collaborator

See here: libgit2/libgit2#2687
Doing this speeds things up from around 2500-3500ms for a small-medium repo to around 180-200ms. We should remove this once it's fixed in libgit2, but until then, this seems silly not to add.

This also adds pathspec.create (since we need that to actually compare pathspecs)

@maxkorp
maxkorp force-pushed the speedup-index-addall branch from 9417492 to a8fd045 Compare March 9, 2015 18:11
@maxkorp
maxkorp force-pushed the speedup-index-addall branch from 0180b9f to 5afb809 Compare March 9, 2015 22:26
Comment thread lib/index.js Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this just be: return paths;?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. I don't know the purpose of the Promise.resolve

@johnhaley81

Copy link
Copy Markdown
Collaborator

Are we ok with putting this in 0.3.0 milestone?

@tbranyen

Copy link
Copy Markdown
Member

Works for me.

@johnhaley81

Copy link
Copy Markdown
Collaborator

I'm going to take out that Promise.resolve(paths) thing first.

@johnhaley81 johnhaley81 changed the title make index.addAll use status to speed up Make index.addAll use status to increase performance Mar 10, 2015
johnhaley81 added a commit that referenced this pull request Mar 10, 2015
Make index.addAll use status to increase performance
@johnhaley81
johnhaley81 merged commit 6352da1 into master Mar 10, 2015
@johnhaley81
johnhaley81 deleted the speedup-index-addall branch March 10, 2015 17:35
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.

3 participants