Skip to content
This repository has been archived by the owner on Dec 18, 2017. It is now read-only.

#2 : Low hanging fruit performance improvements from profiling session #1188

Merged
merged 1 commit into from
Feb 11, 2015
Merged

#2 : Low hanging fruit performance improvements from profiling session #1188

merged 1 commit into from
Feb 11, 2015

Conversation

Praburaj
Copy link
Contributor

Bug #1147

Addresses:
ResolvePackagePath reads sha512 files from the disk when it doesn't need to.
Regex use in SemanticVersion and VersionUtility.ParseFrameworkName should be removed. Regex has been replaced with regular string comparison in few more places. Now except PathResolver there are no more Regex usages in xre - that will probably be removed with new globbing.

Will plan to add a few unit tests with the next checkin.

/CC @davidfowl

@ghost ghost added the cla-not-required label Feb 10, 2015
@Praburaj
Copy link
Contributor Author

@muratg

@@ -222,11 +222,8 @@ public void Initialize(IEnumerable<LibraryDescription> packages, FrameworkName t
{
var defaultHashPath = defaultResolver.GetHashPath(package.Id, package.Version);
Copy link
Member

Choose a reason for hiding this comment

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

This change isn't good enough. Take a look at the performance branch. It avoids any disk IO in the most common case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see. I will check that out.

Copy link
Contributor

Choose a reason for hiding this comment

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

what's the strategy for not reading sha512? are we putting the base64 hash bytes in the lock.json written by restore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had a look at the perfomance_stuff branch change on this. I probably need to understand this a bit better. It looks like you still do a disk read to read the contents of the file in for loop during the first time. Am I missing something here?

Copy link
Member

Choose a reason for hiding this comment

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

@lodejard We're going to.

@Praburaj
Copy link
Contributor Author

@davidfowl - updated PR.

Bug# #1147

Addresses:
ResolvePackagePath reads sha512 files from the disk when it doesn't need to.
Regex use in SemanticVersion and VersionUtility.ParseFrameworkName should be removed. Regex has been replaced with regular string comparison in few more places. Now except PathResolver there are no more Regex usages in xre - that will probably be removed with new globbing.
@davidfowl
Copy link
Member

:shipit:

@Praburaj Praburaj merged commit e234335 into aspnet:dev Feb 11, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants