Skip to content
This repository was archived by the owner on Aug 8, 2022. It is now read-only.

Commit ec15447

Browse files
author
Daniel Jette
committed
Account for uneven chunk sizes
The original code could not handle any case when the number of processing chunks cannot evenly divide the pixel buffer
1 parent d557bc8 commit ec15447

6 files changed

Lines changed: 128 additions & 21 deletions

File tree

‎.github/workflows/library_build.yml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,3 +67,10 @@ jobs:
6767
# Run test Gradle task
6868
- name: Run JVM Tests
6969
run: ./gradlew :Library:test
70+
71+
- name: Archive test results
72+
if: ${{ always() }}
73+
uses: actions/upload-artifact@v2
74+
with:
75+
name: junit-test-results
76+
path: ./Library/build/reports/tests/**/**

‎Library/build.gradle‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ android {
6666
implementation "androidx.test:runner:${versions.androidx.test.runner}"
6767
implementation "org.jetbrains.kotlin:kotlin-stdlib-jdk8:${versions.kotlin}"
6868
implementation "com.github.ajalt:colormath:${versions.colormath}"
69-
implementation("org.jetbrains.kotlinx:kotlinx-coroutines-android:1.4.3") {
69+
implementation("org.jetbrains.kotlinx:kotlinx-coroutines-android:${versions.kotlinx}") {
7070
exclude group: 'org.jetbrains.kotlin', module: 'kotlin-stdlib-jdk8'
7171
exclude group: 'org.jetbrains.kotlin', module: 'kotlin-stdlib-jdk7'
7272
exclude group: 'org.jetbrains.kotlin', module: 'kotlin-stdlib'
@@ -75,6 +75,7 @@ android {
7575

7676
testImplementation "org.mockito:mockito-core:${versions.mockito2}"
7777
testImplementation "com.nhaarman.mockitokotlin2:mockito-kotlin:${versions.mockitokotlin}"
78+
testImplementation "org.jetbrains.kotlinx:kotlinx-coroutines-test:${versions.kotlinx}"
7879

7980
androidTestImplementation "androidx.test.ext:junit:${versions.androidx.test.junit}"
8081
androidTestImplementation "androidx.test:runner:${versions.androidx.test.runner}"

‎Library/src/main/java/com/shopify/testify/internal/processor/BitmapExtentions.kt‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package com.shopify.testify.internal.processor
22

33
import android.graphics.Bitmap
4+
import androidx.annotation.VisibleForTesting
5+
import kotlinx.coroutines.CoroutineDispatcher
46
import kotlinx.coroutines.asCoroutineDispatcher
57
import java.util.concurrent.Executors
68

@@ -13,7 +15,17 @@ fun ParallelPixelProcessor.TransformResult.createBitmap(): Bitmap {
1315
)
1416
}
1517

16-
var numberOfCores = Runtime.getRuntime().availableProcessors()
18+
private val numberOfAvailableCores = Runtime.getRuntime().availableProcessors()
19+
20+
@VisibleForTesting
21+
var maxNumberOfChunkThreads = numberOfAvailableCores
22+
23+
@VisibleForTesting
24+
var _executorDispatcher: CoroutineDispatcher? = null
25+
1726
val executorDispatcher by lazy {
18-
Executors.newFixedThreadPool(numberOfCores).asCoroutineDispatcher()
27+
if (_executorDispatcher == null) {
28+
_executorDispatcher = Executors.newFixedThreadPool(numberOfAvailableCores).asCoroutineDispatcher()
29+
}
30+
_executorDispatcher!!
1931
}

‎Library/src/main/java/com/shopify/testify/internal/processor/ParallelPixelProcessor.kt‎

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ class ParallelPixelProcessor private constructor() {
4545

4646
private fun getChunkData(width: Int, height: Int): ChunkData {
4747
val size = width * height
48-
val chunkSize = size / numberOfCores
48+
val chunkSize = size / maxNumberOfChunkThreads
4949
val chunks = ceil(size.toFloat() / chunkSize.toFloat()).toInt()
5050
return ChunkData(size, chunks, chunkSize)
5151
}
@@ -55,7 +55,9 @@ class ParallelPixelProcessor private constructor() {
5555
launch(executorDispatcher) {
5656
(0 until chunkData.chunks).map { chunk ->
5757
async {
58-
for (i in (chunk * chunkData.chunkSize) until ((chunk + 1) * chunkData.chunkSize)) {
58+
val start = chunk * chunkData.wholeChunkSize
59+
val end = start + chunkData.getSizeOfChunk(chunk)
60+
for (i in start until end) {
5961
if (!fn(chunk, i)) break
6062
}
6163
}
@@ -113,8 +115,17 @@ class ParallelPixelProcessor private constructor() {
113115
private data class ChunkData(
114116
val size: Int,
115117
val chunks: Int,
116-
val chunkSize: Int
117-
)
118+
val wholeChunkSize: Int
119+
) {
120+
fun getSizeOfChunk(chunk: Int) = if (isLastChunk(chunk) && isUnevenChunkSize()) {
121+
size % wholeChunkSize
122+
} else {
123+
wholeChunkSize
124+
}
125+
126+
private fun isLastChunk(chunk: Int) = (chunk == chunks - 1)
127+
private fun isUnevenChunkSize(): Boolean = (size % wholeChunkSize != 0)
128+
}
118129

119130
private data class ImageBuffers(
120131
val width: Int,

‎Library/src/test/java/com/shopify/testify/ParallelPixelProcessorTest.kt‎

Lines changed: 89 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,63 +1,138 @@
11
package com.shopify.testify
22

33
import android.graphics.Bitmap
4+
import com.nhaarman.mockitokotlin2.any
45
import com.nhaarman.mockitokotlin2.doReturn
56
import com.nhaarman.mockitokotlin2.mock
67
import com.nhaarman.mockitokotlin2.whenever
78
import com.shopify.testify.internal.processor.ParallelPixelProcessor
8-
import com.shopify.testify.internal.processor.numberOfCores
9+
import com.shopify.testify.internal.processor._executorDispatcher
10+
import com.shopify.testify.internal.processor.maxNumberOfChunkThreads
11+
import kotlinx.coroutines.Dispatchers
12+
import kotlinx.coroutines.ExperimentalCoroutinesApi
13+
import kotlinx.coroutines.ObsoleteCoroutinesApi
14+
import kotlinx.coroutines.newSingleThreadContext
15+
import kotlinx.coroutines.test.resetMain
16+
import kotlinx.coroutines.test.setMain
17+
import org.junit.After
918
import org.junit.Assert.assertEquals
19+
import org.junit.Assert.assertTrue
1020
import org.junit.Before
1121
import org.junit.Test
22+
import java.util.concurrent.atomic.AtomicInteger
1223

24+
@ObsoleteCoroutinesApi
25+
@ExperimentalCoroutinesApi
1326
class ParallelPixelProcessorTest {
1427

28+
private val mainThreadSurrogate = newSingleThreadContext("UI thread")
29+
1530
companion object {
1631
const val WIDTH = 1080
1732
const val HEIGHT = 2220
1833
}
1934

20-
private fun mockBitmap(): Bitmap {
35+
private fun mockBitmap(width: Int = WIDTH, height: Int = HEIGHT): Bitmap {
2136
return mock<Bitmap>().apply {
22-
doReturn(WIDTH).whenever(this).height
23-
doReturn(HEIGHT).whenever(this).width
37+
doReturn(width).whenever(this).height
38+
doReturn(height).whenever(this).width
39+
doReturn(0xffffffff.toInt()).whenever(this).getPixel(any(), any())
2440
}
2541
}
2642

27-
private val expectedX = (0 until WIDTH).flatMap { (0 until HEIGHT).toList() }
28-
private val expectedY = (0 until WIDTH).flatMap { y -> (0 until HEIGHT).map { y } }
2943
private lateinit var pixelProcessor: ParallelPixelProcessor
3044

45+
private fun forceSingleThreadedExecution() {
46+
Dispatchers.setMain(mainThreadSurrogate)
47+
_executorDispatcher = Dispatchers.Main
48+
}
49+
3150
@Before
3251
fun setUp() {
52+
forceSingleThreadedExecution()
3353
pixelProcessor = ParallelPixelProcessor
3454
.create()
3555
.baseline(mockBitmap())
3656
.current(mockBitmap())
3757
}
3858

59+
@After
60+
fun tearDown() {
61+
Dispatchers.resetMain()
62+
mainThreadSurrogate.close()
63+
}
64+
3965
@Test
4066
fun default() {
41-
numberOfCores = 1
67+
maxNumberOfChunkThreads = 1
68+
69+
val index = AtomicInteger(0)
70+
pixelProcessor.analyze { _, _, _ ->
71+
index.incrementAndGet()
72+
true
73+
}
74+
assertEquals(WIDTH * HEIGHT, index.get())
75+
}
76+
77+
@Test
78+
fun twoCores() {
79+
maxNumberOfChunkThreads = 2
80+
81+
val index = AtomicInteger(0)
82+
pixelProcessor.analyze { _, _, _ ->
83+
index.incrementAndGet()
84+
true
85+
}
86+
87+
assertEquals(WIDTH * HEIGHT, index.get())
88+
}
89+
90+
@Test
91+
fun oddNumberOfCores() {
92+
maxNumberOfChunkThreads = 7
93+
94+
val index = AtomicInteger(0)
95+
pixelProcessor.analyze { _, _, _ ->
96+
index.incrementAndGet()
97+
true
98+
}
99+
100+
assertEquals(WIDTH * HEIGHT, index.get())
101+
}
102+
103+
@Test
104+
fun oddNumberOfPixels() {
105+
maxNumberOfChunkThreads = 2
106+
107+
pixelProcessor = ParallelPixelProcessor
108+
.create()
109+
.baseline(mockBitmap(3, 3))
110+
.current(mockBitmap(3, 3))
111+
112+
val expected = mutableSetOf(
113+
0 to 0, 1 to 0, 2 to 0,
114+
0 to 1, 1 to 1, 2 to 1,
115+
0 to 2, 1 to 2, 2 to 2
116+
)
42117

43-
var index = 0
118+
val index = AtomicInteger(0)
44119
pixelProcessor.analyze { _, _, (x, y) ->
45-
assertEquals(expectedX[index], x)
46-
assertEquals(expectedY[index], y)
47-
index++
120+
assertTrue(expected.remove(x to y))
121+
index.incrementAndGet()
48122
true
49123
}
124+
assertEquals(9, index.get())
125+
assertTrue(expected.isEmpty())
50126
}
51127

52128
private fun assertPosition(index: Int, position: Pair<Int, Int>) {
53-
val width = 1080
54-
val (x, y) = pixelProcessor.getPosition(index, width)
129+
val (x, y) = pixelProcessor.getPosition(index, WIDTH)
55130
assertEquals(position, x to y)
56131
}
57132

58133
@Test
59134
fun multicoreChunks() {
60-
numberOfCores = 2
135+
maxNumberOfChunkThreads = 2
61136
assertPosition(7, 7 to 0)
62137
assertPosition(500, 500 to 0)
63138
assertPosition(1500, 420 to 1)

‎build.gradle‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ buildscript {
2323
'colormath' : '1.4.1', // https://github.com/ajalt/colormath/releases
2424
'dokka' : '1.4.32', // https://github.com/Kotlin/dokka/releases
2525
'kotlin' : '1.4.31', // https://kotlinlang.org/
26+
'kotlinx' : '1.4.3', // https://github.com/Kotlin/kotlinx.coroutines/releases
2627
'ktlint' : '0.36.0', // https://github.com/pinterest/ktlint
2728
'material' : '1.3.0', // https://material.io/develop/android/docs/getting-started/
2829
'mockito2' : '3.3.3', // https://github.com/mockito/mockito/releases

0 commit comments

Comments
 (0)