Community
Participate
Working Groups
The following patch will improve performance of registering service and changing properties of registered services ( more precisely speaking, create properties). ( I don't think it improves so much but surely.)
Created attachment 45559 [details] Patch of ServiceRegistrationImpl
Seems we are really doing very minor enhancements now... :-) Anyway, there is a problem in the patch. + properties.set(Constants.OBJECTCLASS, clazzes,true); must be changed to + properties.set(Constants.OBJECTCLASS, cloneValue(clazzes), true); You must not expose the internal clazzes array to the bundles since arrays are mutable and a bundle can alter the elements of the internal array. You must clone the array and then present the cloned array to the bundles. This problem does not exist for service.id since Long is immutable.
Created attachment 45575 [details] Patch of ServiceRegistrationImpl(revised) Thanks BJ. What you said is right. The attached one is revised. I'll do performance tests against codes with this patch for confirmation ( although I guess the contribution in terms of performance will be a little.)
I did my performance tests which both the first patch and the second of Bug#149030 and the latest patch of Bug#149248(This bug) are applied. https://bugs.eclipse.org/bugs/show_bug.cgi?id=149030 https://bugs.eclipse.org/bugs/show_bug.cgi?id=149248 First of all, I currently have a problem in my performance tests that results vary. --------------------------------------------------------------- Registering 10,000 services takes totally (a) (b) (c) With 3 props: 6.617 -> 6.494 -> 6.299[sec] With no props: 5.051 -> 4.895 -> 4.917[sec] (a) before fix of Bug#149030 (b) after fix of Bug#149030 (c) after fix of Bug#149030 and Bug#149248 --------------------------------------------------------------- Although there are inversion between (b) and (c) unfortunately, it seems within the margin of error. I believe these patches improve performance of not getServiceReference()/getService(reference) but registerService(). P.S. I cannot see difference for getService(ref)/getServiceReference().
Excuse me if I'm a dolt for asking this question Ikuo, but would it be possible fo r you to share (possibly on the Equinox Wiki) on how you perform these tests? I love the work and it would be great if it could be repeated by others :)
Hi Chris, thank you for your comment. Hmm, it's possible I think. But it takes time. Currently, I run my test code extending PerformanteTestRunner. In order to run each test independently, I can submit my test codes to share among others (it is possible actually). However, Since I would like to run multiple tests continuously (and effectively) and save each results into csv format under my own specific name, I took some tricky but not sophisticated ways; # I usually run tests during day and night; It takes so much time.... # I need to be patient sometimes :-) 1. I modified "org.eclipse.test.internal.performance.InternalPerformanceMeter" in "org.eclipse.test.performance" PI by myself to produce a result file under static file name (e.g. "results.csv") at meter.commit(). 2. After PerformanceTestRunner.run(), my code renames the file produced "result.csv" into specific name, e.g. "result_NoSt_DMa_5000_5_i8_100.csv", which represents "No use of ServifceTracker, type = DMa, number of types of classes = 5,000, number of registration duplication =5, inner = 8, outer =100". (I don't use Derby DB now) 3. The last test(method) in my test class calculate average values from these files under specified directory and produce "summary.csv". Anyway, Jeff, Oleg and I had talked that some of my tests should be included in AutomatedTest before (We didn't start to merge them yet). We'll discuss this topic. # Posting my codes on wiki might not be permanent solution # but it is fine for instant one.
(In reply to comment #2) BJ your statement about the need for cloneValue(clazzes) is incorrect. Using the internal classes[] as a value in the internal properties object will not expose the array to other bundles. The only way bundles may access the properties values is by calling ServiceReference.getProperty method. This method perperly calls cloneValue on any values returned.
Comment on attachment 45559 [details] Patch of ServiceRegistrationImpl Making patch valid per last comment.
Comment on attachment 45575 [details] Patch of ServiceRegistrationImpl(revised) making patch obsolete per comment 7.
(In reply to comment #7) > (In reply to comment #2) > > BJ your statement about the need for cloneValue(clazzes) is incorrect. Using > the internal classes[] as a value in the internal properties object will not > expose the array to other bundles. The only way bundles may access the > properties values is by calling ServiceReference.getProperty method. This > method perperly calls cloneValue on any values returned. > Yes. Normally properties are cloned on the way "in" to avoid side effects from the "setter" and cloned on the way "out" to avoid side effects from the "getter". But since the framework is the setter of the clazzes array, we can assume that there will be no side effects from the setter. If the framework gets the array directly from the field, then we must also trust the framework to not cause side effects as the getter.
(In reply to comment #6) > Anyway, Jeff, Oleg and I had talked that some of my tests should be included in > AutomatedTest before (We didn't start to merge them yet). We'll discuss this > topic. I opened a new report and posted my test codes to it. Please check it if you are interested in it. https://bugs.eclipse.org/bugs/show_bug.cgi?id=150872
[contributed patch applied] The fix is safe. I went ahead and released it into the 3.3 stream.
adding "contributed" keyword to patches contributed by the community.
