Optimize singletons and companion objects (resolves bitfireAT/davx5#25) (3c6d1172) · Commits · e / os / AccountManager

app/src/main/java/at/bitfire/davdroid/HttpClient.kt

+13 −8

Original line number	Diff line number	Diff line
		@@ -44,28 +44,33 @@ class HttpClient private constructor(
		/** max. size of disk cache (10 MB) */
		const val DISK_CACHE_MAX_SIZE: Long = 1010241024

		/** [OkHttpClient] singleton to build all clients from */
		val sharedClient: OkHttpClient = OkHttpClient.Builder()
		/** Base Builder to build all clients from. Use rarely; [OkHttpClient]s should
		* be reused as much as possible. */
		fun baseBuilder() =
		OkHttpClient.Builder()
		// Set timeouts. According to [AbstractThreadedSyncAdapter], when there is no network
		// traffic within a minute, a sync will be cancelled.
		.connectTimeout(15, TimeUnit.SECONDS)
		.writeTimeout(30, TimeUnit.SECONDS)
		.readTimeout(120, TimeUnit.SECONDS)
		.pingInterval(45, TimeUnit.SECONDS) // avoid cancellation because of missing traffic; only works for HTTP/2
		.pingInterval(
		45,
		TimeUnit.SECONDS
		) // avoid cancellation because of missing traffic; only works for HTTP/2

		// keep TLS 1.0 and 1.1 for now; remove when major browsers have dropped it (probably 2020)
		.connectionSpecs(listOf(
		.connectionSpecs(
		listOf(
		ConnectionSpec.CLEARTEXT,
		ConnectionSpec.COMPATIBLE_TLS
		))
		)
		)

		// don't allow redirects by default, because it would break PROPFIND handling
		.followRedirects(false)

		// add User-Agent to every request
		.addInterceptor(UserAgentInterceptor)

		.build()
		}


		@@ -87,7 +92,7 @@ class HttpClient private constructor(
		// default cookie store for non-persistent cookies (some services like Horde use cookies for session tracking)
		private var cookieStore: CookieJar? = MemoryCookieStore()

		private val orig = sharedClient.newBuilder()
		private val orig = baseBuilder()

		init {
		// add network logging, if requested

app/src/main/java/at/bitfire/davdroid/Singleton.kt

+32 −9

Original line number	Diff line number	Diff line
		@@ -11,16 +11,29 @@ import java.lang.ref.WeakReference
		*/
		object Singleton {

		private val currentlyCreating = HashSet<Class<*>>()
		private val currentlyCreating = HashSet<Any>()

		private val singletons = mutableMapOf<Any, WeakReference<Any>>()


		/**
		* Gets a singleton instance of the class, using the Class [T] as key.
		*
		* This method is thread-safe.
		*/
		inline fun<reified T> getInstance(noinline createInstance: () -> T): T =
		getInstance(T::class.java, createInstance)
		getInstanceByKey(T::class.java, createInstance)

		/**
		* Gets a singleton instance of the class, using the Class [T] as key.
		*
		* Accepts an Android Context, which is used to determine the [Context.getApplicationContext].
		* The application Context is then passed to [createInstance].
		*
		* This method is thread-safe.
		*/
		inline fun<reified T> getInstance(context: Context, noinline createInstance: (appContext: Context) -> T): T =
		getInstance(T::class.java) {
		getInstanceByKey(T::class.java) {
		createInstance(context.applicationContext)
		}

		@@ -30,9 +43,19 @@ object Singleton {
		singletons.clear()
		}

		/**
		* Gets a singleton instance of the class (using a given key).
		*
		* This method is thread-safe.
		*
		* @param key unique key (only one instance is created for this key)
		* @param createInstance creates the instance
		*
		* @throws IllegalStateException when called recursively with the same key
		*/
		@Synchronized
		fun<T> getInstance(clazz: Class<T>, createInstance: () -> T): T {
		var cached = singletons[clazz]
		fun<T> getInstanceByKey(key: Any, createInstance: () -> T): T {
		var cached = singletons[key]
		if (cached != null && cached.get() == null) {
		singletons.remove(cached)
		cached = null
		@@ -45,16 +68,16 @@ object Singleton {

		// CREATE NEW SINGLETON
		// prevent recursive creation
		if (currentlyCreating.contains(clazz))
		if (currentlyCreating.contains(key))
		throw IllegalStateException("Singleton.getInstance must not be called recursively")
		currentlyCreating += clazz
		currentlyCreating += key
		// actually create the instance
		try {
		val newInstance = createInstance()
		singletons[clazz] = WeakReference(newInstance)
		singletons[key] = WeakReference(newInstance)
		return newInstance
		} finally {
		currentlyCreating -= clazz
		currentlyCreating -= key
		}
		}

app/src/main/java/at/bitfire/davdroid/syncadapter/SyncAdapterService.kt

+1 −12

Original line number	Diff line number	Diff line
		@@ -19,6 +19,7 @@ import androidx.core.content.getSystemService
		import at.bitfire.davdroid.ConcurrentUtils
		import at.bitfire.davdroid.InvalidAccountException
		import at.bitfire.davdroid.PermissionUtils
		import at.bitfire.davdroid.Singleton
		import at.bitfire.davdroid.log.Logger
		import at.bitfire.davdroid.settings.AccountSettings
		import at.bitfire.davdroid.ui.account.WifiPermissionsActivity
		@@ -63,18 +64,6 @@ abstract class SyncAdapterService: Service() {
		* Useful if settings which modify parsing/local behavior have been changed.
		*/
		const val SYNC_EXTRAS_FULL_RESYNC = "full_resync"

		/**
		* We use our own dispatcher to
		*
		* - make sure that all threads have [Thread.getContextClassLoader] set, which is required for dav4jvm and ical4j (because they rely on [ServiceLoader]),
		* - control the global number of sync worker threads.
		*
		* Threads created by a service automatically have a contextClassLoader.
		*/
		val workDispatcher =
		ThreadPoolExecutor(0, Integer.min(Runtime.getRuntime().availableProcessors(), 6),
		10, TimeUnit.SECONDS, LinkedBlockingQueue()).asCoroutineDispatcher()
		}

app/src/main/java/at/bitfire/davdroid/syncadapter/SyncManager.kt

+20 −2

Original line number	Diff line number	Diff line
		@@ -53,6 +53,8 @@ import java.net.HttpURLConnection
		import java.security.cert.CertificateException
		import java.util.*
		import java.util.concurrent.LinkedBlockingQueue
		import java.util.concurrent.ThreadPoolExecutor
		import java.util.concurrent.TimeUnit
		import java.util.concurrent.atomic.AtomicInteger
		import java.util.logging.Level
		import javax.net.ssl.SSLHandshakeException
		@@ -97,6 +99,22 @@ abstract class SyncManager<ResourceType: LocalResource<*>, out CollectionType: L

		protected var hasCollectionSync = false

		/**
		* We use our own dispatcher to
		*
		* - make sure that all threads have [Thread.getContextClassLoader] set, which is required for dav4jvm and ical4j (because they rely on [ServiceLoader]),
		* - control the global number of sync worker threads.
		*
		* Threads created by a service automatically have a contextClassLoader.
		*/
		val workDispatcher = Singleton.getInstanceByKey("SyncManager.workDispatcher") {
		ThreadPoolExecutor(
		0, Integer.min(Runtime.getRuntime().availableProcessors(), 6),
		10, TimeUnit.SECONDS, LinkedBlockingQueue()
		).asCoroutineDispatcher()
		}


		override fun close() {
		httpClient.close()
		}
		@@ -321,7 +339,7 @@ abstract class SyncManager<ResourceType: LocalResource<*>, out CollectionType: L
		var numUploaded = 0

		// upload dirty resources (parallelized)
		runBlocking(SyncAdapterService.workDispatcher) {
		runBlocking(workDispatcher) {
		for (local in localCollection.findDirty())
		launch {
		localExceptionContext(local) {
		@@ -510,7 +528,7 @@ abstract class SyncManager<ResourceType: LocalResource<*>, out CollectionType: L
		}
		}

		withContext(SyncAdapterService.workDispatcher) { // structured concurrency: blocks until all inner coroutines are finished
		withContext(workDispatcher) { // structured concurrency: blocks until all inner coroutines are finished
		listRemote { response, relation ->
		// ignore non-members
		if (relation != Response.HrefRelation.MEMBER)