Community
Participate
Working Groups
In BundleContextImpl#registerService(clazzes, service, prop) , clazzes are copied into String[] copy as follows: - - - - - - - - - - -[Current code]- - - - - - - - - - - /* copy the array so that changes to the original will not affect us. */ String[] copy = new String[clazzes.length]; for (int i = 0; i < clazzes.length; i++) { copy[i] = new String(clazzes[i].getBytes()); } clazzes = copy; - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - I wonder if the following implementation is enough. - - - - - - - - - - -[Revised code]- - - - - - - - - - - /* copy the array so that changes to the original will not affect us. */ String[] copy = new String[clazzes.length]; for (int i = 0; i < clazzes.length; i++) { copy[i] = clazzes[i]; } clazzes = copy; - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - If my understanding is correct, it improves performance of registerService() because both of String#getByte() and new String() are costly. Roughly speaking, registering 10,000 services takes under a condition (on Windows XP (Pen4 2.5GHz), JVMOPTIONS=-Xms256M -Xmx256M -Xint): [current] 12.94 sec [revised] 11.03 sec # Remember the required time may change according to other stuff # ( e.g. the number of ServiceListeners registered).
Created attachment 42793 [details] Patch to improve performance of service registration
Yes, this would be a better code as well. You might want to go one step further and replace the cycle with: System.arraycopy(clazzes, 0, copy, 0, clazzes.length);
I think the String "copy" code came from when the code base was working with J9 resource management which used multiple heaps. The goal of the code was to "move" the String objects into the heap of the framework from the heap of the calling bundle. This code should have been isolated into a method call to better document its purpose. Since J9 resource management support is not a stated goal of Equinox at this time, I agree that System.arraycopy is the best choice.
Created attachment 44955 [details] The latest patch of BundleContextImpl Thanks Oleg and BJ. I'm sorry I have forgotten to post the latest patch. Please close the bug when it is committed. >> committers
Tom, I will have to assume you meant 3.3M1 since 3.2M1 was a looong time ago :-) +1 for updated patch.
(In reply to comment #0) > [current] 12.94 sec > [revised] 11.03 sec > Ikuo, Do you have new performance numbers for the updated patch vs. original?
opps, yes 3.3 M1 ;-) I released the latest patch to HEAD. Marking as fixed. Ikuo, please provide any new performance numbers you have with the latest fix. Also re-open this bug if you find additional areas where service registration performance could be improved. Thanks [contributed patch applied]
(In reply to comment #6) > (In reply to comment #0) > > [current] 12.94 sec > > [revised] 11.03 sec I have recently change my test implementation mainly on the way to load multiple classes (and interfaces). While the old one has fragmented bundle which actually contains 10,000 interfaces and 10,000 implementing classes and they are loaded, the new one has no fragmented bundle which contains 1 interface and 1 class and it can dynamically produce specified number of interfaces with different name and its implementing classes. In both, service objects are created and stored stored in List, and interface names are stored in String[] before TestRunner.run(). In test(), of which CPU time is measured, these objects and interface name String are retrieved from List and String[]. Although I expected that it would affect the results, it seems it did. In other words, the new one is faster than the old. (I didn't investigate this reason...It might be caused other implementation problem in old one.) # Therefore, I'm now doing many tests again using new one. Anyway, one of the results I have now are as follows: -------- Registering 10,000 services with 3 properties takes (in Interpreted Mode) [osgi_3.2.0.N20040424-0010.jar + fix of Bug#142385] 9.24 s [Latest* + fix of Bug#142385] 6.72 s *Fixed bugs in Latest include not only Bug#144055 but also Bug#144540. -------- It will be much faster when using Hotspot, needless to say.
(In reply to comment #8) > I have recently change my test implementation mainly on the way to load > multiple classes (and interfaces). While the old one has fragmented bundle > which actually contains 10,000 interfaces and 10,000 implementing classes and > they are loaded, the new one has no fragmented bundle which contains 1 > interface and 1 class and it can dynamically produce specified number of > interfaces with different name and its implementing classes. Oh, I should have comment who contributed to new excellen original code, which dynamically creates classes and interfaces, for me. It was Oleg. Thank you so much !!
adding "contributed" keyword to patches contributed by the community.
