Hi,
Eric Kow <[email protected]> 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)
Yours,
Petr.
_______________________________________________
darcs-users mailing list
[email protected]
http://lists.osuosl.org/mailman/listinfo/darcs-users
