David E Jones wrote: > [snip] You know, I was going to start a thread like this, but you beat me to it. I actually have a concrete example of a major problem, so I'm going to chime in.
As many are aware, I have been going thru adding many test cases to lots of low-level utility code. *Every* time I have done so, I have *always* found a problem. Without fail. It's not that A with B with C with D fails. It's that A and B and C and D have issues all by themselves. I consider this extremely unacceptable. The latest problem I have is quite the complex. It's the DelegatorFactory change. It's very broken. The original code only existed in GenericDelegator.getGenericDelegator(). It would check a local map, looking for an already constructed delegator, returning *very* fast if one existed. However, the new code inverts this. Instead, DelegatorFactory calls directly into UtilObject.getObjectFromFactory. No caching at all. The caching is deferred to DelegatorFactoryImpl. So far, things seem perfectly fine. However, there are parts of ofbiz that don't store delegator instances. Instead, they store only the delegatorName. org.ofbiz.entity.cache.AbstractCache is one of these. It calls DelegatorFactory.getDelegator every time it needs the actual delegator instance. So, with this long explanation, I still haven't shown any problem. But just wait. ServiceLoader, which is used by getObjectFromFactory, will walk the *entire* classpath, each and every time it is called. Again, still no exact problem. But those who have been following may be starting to see the trees thru this forest. I had an importer written for a client, to convert a legacy database to ofbiz schema. On our internal ofbiz package based on 811524, this importer ran in 15 minutes. However, when I upgrade to 902021, I didn't do a full db drop, and reimport. When I did so today, I had to kill the importer after running for 2 hours, with no end in site. This was due to the entity cache for ProductAssoc getting large, and creation of new ProductAssoc, which was causing the AbstractCache to try and load delegator instances, which was then going thru the entire classpath. This completely fell over. This slowdown is completely unacceptable. However, even once identified, the problem is not straight forward. Caching at a higher level, in DelegatorFactory.getDelegator, is not correct either. Moving the caching higher implies that the constructed delegator won't be saved until it is completely done being constructed. However, during construction, the delegator creates several helper entities. These include EntityCrypto, and the cache support classes. These classes also still have the same problem of not storing delegator instances, instead just storing a name. So, they try to load a delegator with the correct name, and don't find it, because the original delegator is not done being constructed. But, we still aren't done with this particular problem. When GenericPK and GenericValue get created, they try to set their fields. Setting any field on a GenericEntity requires that it knows it's delegator. However, creation doesn't actually set the delegator. Even the delegator name is null. So any GenericEntity created during delegator instantiation ends up trying to load a delegator named "default", *not* with whatever delegator is actually being created. There are a whole series of changes done over the years that have caused these cascading problems. And this problem is just one that I have personally seen that has caused me to worry greatly. What I am about to propose may sound rather severe, but I would like to see a moratorium on any new features. Test cases and bug fixes only. Ofbiz has gotten *WAY* *TOO* BUGGY*. ps: I'm well on the way to fixing this huge delegator problem, and should have it ready tomorrow.