Skip to content
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

Use ForkJoinPool and provide builder API to limit threads count. #1092

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mstyura
Copy link

@mstyura mstyura commented Aug 12, 2022

This PR provide API to configure thread pool limits use by BoxStore.

I was observed situation in Android application where (due to some other issues in app) application is crashed with OOM. At the moment of crash there were tens of alive threads belong to ObjectBox pool. The numbering suggest that thousands of thread were already created and destroyed by the pool at the moment of crash.
So it would be nice to have an API to limit number of threads allowed in thread pool and minimum number of threads as well.
This of course will not solve original problem with app I was debugging, but will probably help to reveal root problem earlier.

@greenrobot-team
Copy link
Member

greenrobot-team commented Aug 16, 2022

Thanks! Maybe we should also consider setting the maximum size to 64 (but at least number of processors) by default. (It's the default parallelism number for Kotlin's IO scheduler.)

@greenrobot-team greenrobot-team self-assigned this Aug 16, 2022
@mstyura mstyura changed the title Builder API to configure thread pool size. WIP: Builder API to configure thread pool size. Aug 16, 2022
@mstyura mstyura marked this pull request as draft August 16, 2022 20:44
@mstyura mstyura force-pushed the thread-pool-limits branch from 9f2b3e8 to 6865c52 Compare August 16, 2022 22:22
@mstyura
Copy link
Author

mstyura commented Aug 16, 2022

Hi! I like the idea of having sane default limit to number of threads in pool.
I've tried to implement it. I've set default value to small value like 4 and launched unit-tests.
I've started getting RejectedExecutionException, which is not I wanted.
RejectedExecutionException had happen due to usage of SynchronousQueue.
I've tried to use unbounded LinkedBlockingQueue, but with unbounded queue the only corePoolSize make sense.
Basically the meaning of ThreadPoolExecutor with corePoolSize=X and unbounded LinkedBlockingQueue is an executor service which at maximum can run X tasks in parallel.
This is what ForkJoinPool does naturally, so I've decided to rather switch to it and only provide configuration to override maximum degree of parallelism.
There is one drawback is that ForkJoinPool allow changing keep alive thread timeout only on the very recent API level. I think this is acceptable for now to use default, while having sane limit to number of threads.

@mstyura mstyura marked this pull request as ready for review August 16, 2022 22:23
@mstyura mstyura changed the title WIP: Builder API to configure thread pool size. Builder API to configure thread pool size. Aug 16, 2022
@mstyura mstyura changed the title Builder API to configure thread pool size. Use ForkJoinPool and provide builder API to limit threads count. Aug 16, 2022
@greenrobot-team
Copy link
Member

@mstyura Oh sorry, I didn't mean for you to implement this. With "we" I meant the ObjectBox team. But thanks for the investigation!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants