Am 06.03.2013 um 14:37 schrieb Lukas Eder: > Hello Antonio, > > 2013/3/6 Antonio Sanso <[email protected]>: >> Hi Lukas >> >> On Mar 6, 2013, at 11:48 AM, Lukas Eder wrote: >> >>> Hello, >>> >>> I've been stepping through the SlingPostServlet, having a look at its >>> code. I feel that there is a potential "code smell" in some of its >>> methods. As I'm unsure how the Sling project deals with such >>> situations, I felt that posting a mail here before creating an issue >>> might be better. >>> >>> The code I'm talking about is contained in >>> registerPostResponseCreator(), where the cachedPostResponseCreators >>> attribute is initialised. There are two issues here: >>> >>> 1. The cachedPostResponseCreators array is re-initialised every time >>> this method is invoked. >> >> if I read correctly the code this method is called only at bundle activation >> and binding > > Yes, that's true. For instance, when the post-servlet-extensions > sample is loaded through the OSGi console, this code is executed. It's > probably only a minor annoyance for most users that do not have an > overly excessive amount of PostResponseCreators :-)
Its not an issue at all. We don't expect (a) a large, aka >100 or more, number of PostResponseCreator services and (b) unregistration is done through unbindPostResponseCreator. Everything should work fine and now problem should be expected to the system load. This is just regular maintenance. As for ComponentContext.locateService: The result of this call is cached in the ComponentContext. Even if it would not be, the call itself is not too overly expensive. > >> Regards >> >> Antonio >> >>> This leads to an unnecessary O(n^2) >>> initialisation complexity on componentContext.locateService() method. There is no initialization involved here: this just accesses a potential cache in the ComponentContext for the named service or -- if missing from the cache -- calls back to the OSGi framework for the service. Regards Felix >>> I guess that this cache's whole purpose is to avoid expensive >>> locateService() calls, so this should probably be fixed. >>> >>> 2. A first loop initialises this cachedPostResponseCreators array. In >>> a second step, "null" entries are "removed" by copying the array. This >>> behaviour is hard to read from the existing code, which is a bit >>> verbose. In order to avoid the potential for bugs, I'd like to suggest >>> using the (untested) behaviour from the attached patch. Note that >>> other methods, such as registerPostProcessor(), >>> registerNodeNameGenerator() suffers from similar "verbosity problems". >>> >>> Cheers >>> Lukas >> -- Felix Meschberger | Principal Scientist | Adobe
