-
Notifications
You must be signed in to change notification settings - Fork 138
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
AutoProcessor - Load and start bundles in parallel #236
base: master
Are you sure you want to change the base?
Conversation
Any chance for feedback on this one? |
@pskowronek interesting concept. Did you encounter this while creating this PR? |
No, I didn't encounter this. We're using this patch in https://github.com/mucommander/mucommander project since around 6 months, and so far no deadlocks were reported. |
Ok, that's good. I might try it as well in our product and see what happens. I think we had the issue 1 out of 10 startups, and have a script that i can test it with. |
Without having reviewed the full PR, I think it would make sense to at least make this behavior optional and thus configurable, to not break any existing applications. But let me first try it out in my application to see if i run into any issues. |
Yes, it should be configurable.
Superb! Thanks. |
Could you please run this against the OSGi TCKs Spec: |
Good point @stbischof. |
This all happens in main Project. The command line launcher. So this may make it easier. But the startlevel chapters should be respected. Did you read them carefully? The bnd Launcher also does start bundle in parallel. Maybe @pkriens can tell us why. |
@pskowronek i checked how we ran parallel startup earlier, and it was based on code shared in https://felix.apache.org/documentation/subprojects/apache-felix-dependency-manager/reference/thread-model.html. How does this PR work differently than via the dependency manager as outlined above? I assume this PR just starts the OSGi bundles in parallel, where the above starts the services in these bundles in parallel, correct? |
@pskowronek i tried it out and compared startup times against the "regular" build. I'm also running our script to detect startup issues, will let you know the outcome. |
I have the results; startup did fail at least once with the following exception:
Seems like there can be a race condition with the dependency manager, see the stacktrace. |
Can you tell me how many bundles the test app has, and what was the processor you were testing on? |
I don't have the exact count at hand, but in the order of 30-40 bundles i think. |
ok, what type of processor the test was run on - the number of (HT) cores might matter. In muCommander we have around 180 bundles , and since this is desktop app, it cpu have 4 (8 HT) cores at minimum. |
Hi Paul, yes, you are correct, when DM is used in concurrent mode, then services are all started and registered concurrently, so concurrency is achieved at the service level. now, about the exception you reported:
after many attempts, I confirm that I also reproduced the problem. So, from the stacktrace, we have a NPE in this location, meaning that in the previous line (here), I cannot comment on this for the moment. |
It it correct that all your bubdles use onlY bundleactivators and no declarative services? |
Yes |
I don't know what causes this, but maybe adding a while loop on the bundleContext being non null with a thread.sleep(100) would resolve this intermittent timing issue? Our startup times greatly improved by starting services in parallel, but the intermittent NPE cause us to not use it in production. |
A PoC of loading and starting bundles in parallel in AutoProcessor.
This was tried in the following project https://github.com/mucommander/mucommander that is using AutoProcessor to load bundles. For muCommander this gave 2-3 times faster bundle loading (from 2-3s to ~400-800ms on my macbook 2012) - see here mucommander/mucommander@ab376b9 (PR in muCommander: https://github.com/mucommacommits/ab376b984257e79a563a0fa49cc128af9c501e9f).
Can you please review and assess whether the concept could be incorporated into Felix, whether it is safe etc.