Status Update
No update yet.
Comments
du...@google.com
Branch: androidx-master-dev
commit b90079595f33f58fece04026a97faa0d243acdb1
Author: Yuichi Araki <yaraki@google.com>
Date: Wed Sep 18 16:55:49 2019
Change the way to detect mismatch between POJO and query
This fixes cursor mismatch warnings with expandProjection.
Bug: 140759491
Test: QueryMethodProcessorTest
Change-Id: I7659002e5e0d1ef60fc1af2a625c0c36da0664d8
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
M room/compiler/src/test/kotlin/androidx/room/processor/QueryMethodProcessorTest.kt
M room/compiler/src/test/kotlin/androidx/room/testing/TestProcessor.kt
lo...@gmail.com
du...@google.com
Branch: androidx-master-dev
commit bdde5a1a970ddc9007b28de4aa29d60ffa588f08
Author: Yigit Boyar <yboyar@google.com>
Date: Thu Apr 16 16:47:05 2020
Re-factor how errors are dismissed when query is re-written
This CL changes how we handle errors/warnings if query is
re-written.
There was a bug in expandProjection where we would report warnings
for things that Room already fixes automatically (
The solution to that problem (I7659002e5e0d1ef60fc1af2a625c0c36da0664d8)
solved it by deferring validating of columns until after re-write
decision is made. Unfortunately, this required changing PojoRowAdapter
to have a dummy mapping until it is validating, make it hard to use
as it does have a non-null mapping which is not useful.
This CL partially reverts that change and instead rely on the log
deferring logic we have in Context. This way, we don't need to break
the stability of PojoRowAdapter while still having the ability to
drop warnings that room fixes. This will also play nicer when we
have different query re-writing options that can use more information
about the query results.
Bug: 153387066
Bug: 140759491
Test: existing tests pass
Change-Id: I2ec967c763d33d7a3ff02c1a13c6953b460d1e5f
M room/compiler/src/main/kotlin/androidx/room/log/RLog.kt
M room/compiler/src/main/kotlin/androidx/room/processor/QueryMethodProcessor.kt
M room/compiler/src/main/kotlin/androidx/room/solver/TypeAdapterStore.kt
M room/compiler/src/main/kotlin/androidx/room/solver/query/result/PojoRowAdapter.kt
ap...@google.com
Branch: androidx-main
commit 348fa7e78cacc892e2c2cd59c5aaadd34a640c2f
Author: Dustin Lam <dustinlam@google.com>
Date: Wed Dec 09 18:24:18 2020
Add Executor param to PagingData operators for Java
Instead of forcing Java users down the sync path without much control
for execution context, add an Executor argument and remove the -Sync
suffix of the method name.
In cases for Guava, while it's possible to build the executor
into the Future itself, it's still desirable to be able to
control which execution context it receives the result on.
Relnote: "Allow Java callers to use PagingData transform operations in
an async way, by accepting an Executor into transform operator
arguments. All of the -Sync transform operators have the -Sync suffix
removed now, and Kotlin Coroutine users will need to disambiguate by
calling the extension function which accepts a suspending block instead.
All PagingData transformation operators have been moved to extensions
under the static PagingDataTransforms class. Java users will need to
call them via static helpers e.g.,
`PagingDataTransforms.map(pagingData, transform)`
For Kotlin users, the syntax is the same but you'll need to import the
function."
Fixes: 172895919
Test: ./gradlew paging:paging-common:test
Change-Id: If688562d07d96e943b1abfa3690042132db1c4d0
M paging/common/api/current.txt
M paging/common/api/public_plus_experimental_current.txt
M paging/common/api/restricted_current.txt
M paging/common/src/main/kotlin/androidx/paging/PagingData.kt
A paging/common/src/main/kotlin/androidx/paging/PagingDataTransforms.kt
A paging/common/src/main/kotlin/androidx/paging/UiReceiver.kt
M paging/integration-tests/testapp/src/main/java/androidx/paging/integration/testapp/v3/V3ViewModel.kt
M paging/integration-tests/testapp/src/main/java/androidx/paging/integration/testapp/v3room/V3RoomViewModel.kt
M paging/samples/src/main/java/androidx/paging/samples/java/InsertSeparatorsJavaSample.java
M paging/samples/src/main/java/androidx/paging/samples/java/InsertSeparatorsJavaUiModelSample.java
M samples/SupportLeanbackDemos/src/main/java/com/example/android/leanback/PhotoViewModel.java
lo...@gmail.com
Do the suspend versions now run on Dispatchers.Default?
du...@google.com
The awaiting context is normally on main thread iirc, but since they're suspending I don't think it should really matter? I would probably argue it's better to avoid extra potential thread hops.
lo...@gmail.com
map and filter are potentially CPU intensive operations (allocation, search, etc), and Kotlin users will likely not wrap their call with dispatcher switching by default, leading to potential UI jank if the default is the main thread. That's why I think Dispatchers.Default should be the default.
du...@google.com
While I agree, part of the issue is that wrapping withContext(Disaptcher.Default) { } will also make it difficult to fast-forward / control timing when used with test scope / pauseDispatcher from kotlinx-coroutines-test. There isn't a good way for Paging to forward the fetchDispatcher to the transforms to provide the default, since the only Paging3 APIs that actually accept a bg / fetchDispatcher are in the paging-runtime adapter / differ APIs.
lo...@gmail.com
I think having a jank-free app by default is mort important than timing control in testing API, as users will ultimately only experience the former.
Now, I think there's a solution to satisfy both use cases:
Have a context: CoroutineContext parameter that defaults to Dispatchers.Default, which allows people to replace it with EmptyCoroutineContext, or whatever they want (including Dispatchers.Main if they need to sync something on the main thread).
du...@google.com
I think having a jank-free app by default is mort important than timing control in testing API, as users will ultimately only experience the former.
I agree, except in this case we would make it impossible for it to be testable, so the trade-off is not great here.
Now, I think there's a solution to satisfy both use cases: Have a context: CoroutineContext parameter that defaults to Dispatchers.Default, which allows people to replace it with EmptyCoroutineContext, or whatever they want (including Dispatchers.Main if they need to sync something on the main thread).
Adding a param seems not great either. Also what happens if developer wants to use .flowOn {} etc to change the executing context? I think generally Flow operators are not supposed to accept a CoroutineContext, but I could be wrong here. Certainly something to think about though.. I think the real solution here is to try to default .submitData to collect on fetchDispatcher, then report differ events through mainDispatcher, but I haven't tried yet so am not sure how feasible it is.
lo...@gmail.com
If EmptyCoroutineContext is used, flowOn will work as expected.
I agree it'd be best to have the collector collect in the right dispatcher. I'm curious if there's any challenges that poses.
du...@google.com
Right, I meant that it is surprising to need to pass in EmptyCoroutineContext to get .flowOn to work.
Description
Hello,
In the
PagingDataclass, there'sfilterSyncand other functions that accept a blocking lambda, but there's no information in the KDoc about which dispatcher/thread they are called on.I guess they are called on
Dispatchers.Default… is it the case?If so, I'd submit a PR on GitHub for this change on
filterSyncand friends.