> I did a test and it is still pretty easy to reverse merge the
> original performance patch to get things back to the way they
> were when you first got things working. Do you think it
> would be easier for you if we go back to that point and you
> just try the performance patches again from square one? I
> would suggest going right to the areas where you think you
> got the biggest performance gains and skipping the smaller
> changes that didn't make much difference. If nothing else,
> we should try to take them one at a time so we know which
> specific change caused the breakage.
>
> I didn't back it out yet because I want to hear your
> thoughts. I absolutely do not mind continuing the way we
> have been until we get it all working again. It just seems
> like we might have changed too many things in that one commit
> and we perhaps should redo it in smaller pieces. That way,
> if you want get rid of an inner loop we can look at that
> change in isollation. Even if they are not big performance
> gains, better code is better code. Since testing is hard we
> should probably just do them one at a time.
>
> Mark
Hello Mark,
believe me, I'm pretty upset the way things are going, and I also did not
mind to work this way ...
Ther problem is that this sync stuff is really pretty hard to verify.
The unit test fo this mess would be absolutely wonderfull, but I can't
imagine how to write it to guarantee its 100% isolation level.
This last issue, I was not able to reproduce at all, until I completely
deleted the workspace and started from fresh one.
It means the unit test would also need to remove old workspace, set up a new
one etc ...
Is there any way how to achieve it ? I have no idea how ...
The problem itself was indeed related to removal of
PersistantResourceVariantByteStore.
It was his responsibilty to register the synchronization partner and since
it has gone, nobody was setting it.
However, since I had an old workspace the partner was already registered.
Fix is pretty simple: add
ResourcesPlugin.getWorkspace().getSynchronizer().add(qualifiedName); to
constructor.
Like this:
public static synchronized SVNWorkspaceSubscriber getInstance() {
if (instance == null) {
instance = new SVNWorkspaceSubscriber();
ResourcesPlugin.getWorkspace().getSynchronizer().add(qualifiedName);
}
return instance;
}
Regarding starting from square one, I do not think that it is necessary, I
believe the last patch
solved the problems properly, this unregistered partner was just a glitch I
did not encountered in my environment.
I tested it, everything seemed to work fine. I just did not started from
clean workspace.
All right, lesson learned, getting a fresh workspace has to be a part of my
testing scenario from now ...
Last thing, an explanation of this PersistantResourceVariantByteStore story.
Initially I've introduced it as a way how to set the syncInfo on the
resources.
However, later I had to set the syncInfos on the resources directly (by
means of Synchronizer) anyway,
so the original reason for existence of PersistantResourceVariantByteStore
vanished.
Later I've realized that both PersistantResourceVariantByteStore and my code
in SVNWorkspaceSubscriber
were accessing the same syncInfo structures and under some conditions they
were conflicting there.
Thus, since I'm setting the syncInfos a different way, there is no reason
why the PersistantResourceVariantByteStore
should be there at all ...
Regards,
Martin
Received on Fri Jul 1 21:47:33 2005