[darcs-users] [patch257] Resolve issue 1503
me at mornfall.net
Wed Jun 2 15:55:55 UTC 2010
Eric Kow <kowey at darcs.net> writes:
> OK, I think I'm almost ready to apply this (no obvious bugs or things to
> complain about).
>> copyFileUsingCache :: OrOnlySpeculate -> Cache -> HashedDir -> String -> IO ()
>> -copyFileUsingCache oos (Ca cache) subdir f =
>> - do debugMessage $ "I'm doing copyFileUsingCache on "++(hashedDir subdir)++"/"++f
>> +copyFileUsingCache oos (Ca unsortedCache) subdir f =
>> + do let (Ca cache) = Ca (sortBy compareByLocality unsortedCache)
>> + debugMessage $ "I'm doing copyFileUsingCache on "++(hashedDir subdir)++"/"++f
> Interesting. One question: does this sorting belong here? What if in
> the future we want to sort by proximity to the repository directory?
> How would we manage that? What's the right attitude to adopt here? One
> possible answer, for example, is YAGNI. Is that the right one?
I wouldn't say that future-proofing is the problem with this bit of
code. What I find sorely inadequate is that the caches are re-sorted for
every single file that needs to be fetched. Note that with populated
cache and hardlinking, this is a *very* fast operation and the constant
sorting could actually contribute non-negligible amount of time to it.
I think a more global solution that sorts the cache as it is populated
would be more appropriate.
>> fetchFileUsingCachePrivate :: FromWhere -> Cache -> HashedDir -> String -> IO (String, B.ByteString)
>> -fetchFileUsingCachePrivate fromWhere (Ca cache) subdir f =
>> +fetchFileUsingCachePrivate fromWhere (Ca unsortedCache) subdir f =
>> do when (fromWhere == Anywhere) $ copyFileUsingCache ActuallyCopy (Ca cache) subdir f
>> ffuc cache
>> `catchall` debugFail ("Couldn't fetch `"++f++"'\nin subdir "++(hashedDir subdir)++
>> hunk ./src/Darcs/Repository/Cache.hs 265
>> ffuc  = debugFail $ "No sources from which to fetch file `"++f++"'\n"++ show (Ca cache)
>> + Ca cache = Ca (sortBy compareByLocality unsortedCache)
> What's the difference between these two functions?
(This obviously points out the other problem with the "late" sorting:
you need to do it in every function that non-trivially uses the
cache... so it's not just performance, but also maintainability)
More information about the darcs-users