Minimize stat calls #41

Merged
merged 2 commits into from Jan 28, 2013

Conversation

Projects
None yet
2 participants
@andralex
Owner

andralex commented Jan 27, 2013

Minimize stat calls. Also a few refactorings.

+ yap("stat ", workDir);
+ DirEntry workDirEntry;
+ const workDirExists =
+ collectException(workDirEntry = dirEntry(workDir)) is null;

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Jan 28, 2013

Simpler:

const workDirExists = assertNotThrown(workDirEntry = dirEntry(workDir));
@ghost

ghost Jan 28, 2013

Simpler:

const workDirExists = assertNotThrown(workDirEntry = dirEntry(workDir));

This comment has been minimized.

Show comment Hide comment
@andralex

andralex Jan 28, 2013

Owner

That doesn't seem to do what we need here.

@andralex

andralex Jan 28, 2013

Owner

That doesn't seem to do what we need here.

rdmd.d
+ else
+ {
+ yap("mkdir ", workDir);
+ mkdir(workDir);

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Jan 28, 2013

Btw shouldn't this be mkdirRecurse?

@ghost

ghost Jan 28, 2013

Btw shouldn't this be mkdirRecurse?

This comment has been minimized.

Show comment Hide comment
@andralex

andralex Jan 28, 2013

Owner

fixed

rdmd.d
@@ -205,7 +214,8 @@ int main(string[] args)
}
// Have at it
- if (isNewer(root, exe) || anyNewerThan(myDeps.keys, exe))
+ auto exeTime = exe.timeLastModified(SysTime.min);
+ if ((root ~ myDeps.keys).anyNewerThan(exeTime))

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Jan 28, 2013

I'm seeing .keys and concatenation used, but don't we have a way of iterating hash keys lazily?

Perhaps chain(root, myDeps.byKey) (not sure if that works though).

@ghost

ghost Jan 28, 2013

I'm seeing .keys and concatenation used, but don't we have a way of iterating hash keys lazily?

Perhaps chain(root, myDeps.byKey) (not sure if that works though).

This comment has been minimized.

Show comment Hide comment
@andralex

andralex Jan 28, 2013

Owner

fixed

@@ -205,7 +214,8 @@ int main(string[] args)
}
// Have at it
- if (isNewer(root, exe) || anyNewerThan(myDeps.keys, exe))
+ auto exeTime = exe.timeLastModified(SysTime.min);
+ if (chain(root.only, myDeps.byKey).array.anyNewerThan(exeTime))

This comment has been minimized.

Show comment Hide comment
@ghost

ghost Jan 28, 2013

Heheh, .array again. I guess it won't hurt much. anyNewerThan should ideally take a range instead of an array, but let's leave that for some future pull request.

@ghost

ghost Jan 28, 2013

Heheh, .array again. I guess it won't hurt much. anyNewerThan should ideally take a range instead of an array, but let's leave that for some future pull request.

This comment has been minimized.

Show comment Hide comment
@andralex

andralex Jan 28, 2013

Owner

Yah. This script's running time is dominated by system calls and processes spawned. The equivalent written in a traditional scripting language would do plenty more allocation and would be likely a bit slower anyway.

@andralex

andralex Jan 28, 2013

Owner

Yah. This script's running time is dominated by system calls and processes spawned. The equivalent written in a traditional scripting language would do plenty more allocation and would be likely a bit slower anyway.

alexrp added a commit that referenced this pull request Jan 28, 2013

@alexrp alexrp merged commit 4692c52 into dlang:master Jan 28, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment