On Fri, Sep 02, 2005 at 03:40:21AM +0100, Ian Lynagh wrote:
> On Sat, Aug 27, 2005 at 03:37:48PM -0400, David Roundy wrote:
> >
> > [avoid reading patches we just wrote when pulling.
> > David Roundy <[EMAIL PROTECTED]>**20050827192349]
> > <
> > > {
> > hunk ./Pull.lhs 253
> > save_patches _ _ (Just []) = return []
> > save_patches _ _ Nothing = return []
> > save_patches repo opts (Just (p:ps)) =
> > - do p' <- (liftM ppt2pipt) (writePatch repo opts p)
> > + do let pinf = fromJust $ patch2patchinfo p
> > + (_,ptok) <- writePatch repo opts p
> > ps' <- save_patches repo opts $ Just ps
> > hunk ./Pull.lhs 256
> > - return (p':ps')
> > - where ppt2pipt :: (Patch, PatchToken) -> (PatchInfo, PatchToken)
> > - ppt2pipt (patch, pt) = (fromJust (patch2patchinfo patch), pt)
> > + return $ (pinf, ptok) : ps'
> > \end{code}
>
> This looks like it'll mean all the files will stay in memory until their
> patchinfo's get forced later. While we might be able to hack around this
> with seq's, I think it's simpler to stick with the current code.
That's a good point. It would be nice to have some sort of a "sever"
function for PackedStrings, which would copy the memory over to a new
buffer, generating a new PackedString, which would avoid holding lots of
memory just to keep a small substring out of a large PackedString. I guess
you could call it copyPS.
Then we could take the PatchInfo from the original version of the patch and
"copy" it (so that it won't hold the whole patch file in memory), and this
patch would work as I had originally intended.
I'm imagining something like
copyPatchInfo :: PatchInfo -> PatchInfo
which calls
copyPS :: PackedString -> PackedString
copyPS (PS x s l) = createPS l copymem
where copymem pout = withForeignPtr x $ \pin ->
c_memcpy (castPtr pout) (castPtr pin `plusPtr` s)
(fromIntegral l)
> > [use unsafeInterleaveIO in Repository.writePatch to delay opening files.
> > David Roundy <[EMAIL PROTECTED]>**20050827192415
> > This patch solves the "too many open files" bug when pulling many patches.
> > ]
> > )
> > hunk ./Repository.lhs 263
> > writePatch (Repo dir _ (DarcsRepository _)) opts patch =
> > withCurrentDirectory dir $
> > do fp <- DarcsRepo.write_patch opts patch
> > - patch' <- gzReadPatchFileLazily fp
> > + patch' <- unsafeInterleaveIO $
> > + withCurrentDirectory dir $ gzReadPatchFileLazily fp
> > return (patch', DarcsPatchToken fp)
> > writePatch (Repo dir _ GitRepository) _ patch =
> > withCurrentDirectory dir $
> > }
>
> This looks like the way to go to me, although I'd be more comfortable if
> we made the path absolute rather than having the working directory
> change later, when this is forced and we might expect to be in a
> different directory. (maybe gzReadPatchFileLazily should do the
> absolutification and unsafeInterleaveIO itself?)
Sure absolutification would be fine.
I don't think I'd want gzReadPatchFileLazily to do the unsafeInterleaveIO
itself, since you might want to read the file and then immediately delete
it, and it'd be nice for the FastPackedString primitives to allow you to do
that (except on windows...).
> Oh, but make test just finished and it doesn't seem to have solved the
> problem:
Hmmm. I could have sworn this patch would fix this! Alas, no time for me
to track this down today...
--
David Roundy
http://www.darcs.net
_______________________________________________
darcs-devel mailing list
[email protected]
http://www.abridgegame.org/cgi-bin/mailman/listinfo/darcs-devel
