Add multi-thread to ObjcExportHeaderGenerator when processing packageFragments #5564
Conversation
| } | ||
|
|
||
| dependencies { | ||
| implementation(libs.kotlinx.coroutines.core) |
There was a problem hiding this comment.
IIUC, no other part of the compiler uses kotlinx.coroutines. If this is the case, this change effectively adds a new dependency.
Can the parallelization be done using standard JDK means?
For example, another part of the K/N compiler uses java.util.concurrent.Executors:
Maybe it is worth checking other parts of the Kotlin compiler to see what is common.
There was a problem hiding this comment.
I think theres more places that are using them already. This search returns 17 build files with the dependency
There was a problem hiding this comment.
This search returns 17 build files with the dependency
Most of them are not in the compiler.
The ones in the compiler seem to add the dependency because some other dependencies require coroutines. Look for usages of coroutines in the source code, not in the build scripts.
There was a problem hiding this comment.
My bad is this a good example HostExecutor.kt
...rator/impl/k1/src/org/jetbrains/kotlin/backend/konan/objcexport/ObjCExportHeaderGenerator.kt
Outdated
Show resolved
Hide resolved
| val classesToTranslate = mutableListOf<ClassDescriptor>() | ||
|
|
||
| runBlocking { | ||
| packageFragments.forEach { packageFragment -> | ||
| val memberScope = packageFragment.getMemberScope() | ||
|
|
||
| launch(Dispatchers.Default) { | ||
| memberScope.getContributedDescriptors() | ||
| .asSequence() | ||
| .filterIsInstance<CallableMemberDescriptor>() | ||
| .filter { mapper.shouldBeExposed(it) } | ||
| .forEach { | ||
| val classDescriptor = getClassIfCategory(it) | ||
| if (classDescriptor == null) { | ||
| topLevel.getOrPut(it.findSourceFile(), { mutableListOf() }) += it | ||
| } else { | ||
| // If a class is hidden from Objective-C API then it is meaningless | ||
| // to export its extensions. | ||
| if (!classDescriptor.isHiddenFromObjC()) { | ||
| extensions.getOrPut(classDescriptor, { mutableListOf() }) += it | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| val classesToTranslate = mutableListOf<ClassDescriptor>() | ||
|
|
||
| packageFragments.forEach { packageFragment -> | ||
| packageFragment.getMemberScope().collectClasses(classesToTranslate) | ||
| launch(Dispatchers.Default) { | ||
| memberScope.collectClasses(classesToTranslate) | ||
| } | ||
| } |
There was a problem hiding this comment.
ObjCExport header generation process is very order-specific. For example, depending on the translation order, the same declarations may get different names when there are clashes that need mangling.
IIUC, this PR changes the translation order (and therefore is a breaking change) and makes it non-deterministic (which is highly undesirable, as the compiler should produce the same output for the same inputs).
There was a problem hiding this comment.
Hello @SvyatoslavScherbina I think this should solve the order problem WDYT?
There was a problem hiding this comment.
I don't understand how it solves the problem.
Method translation involves translating types. This may in turn require other classes to be translated.
See e.g.
Therefore, the order of method translation may affect the result, even if those methods are unrelated.
…Fragments KT-82436
alfonsocv12
force-pushed
the
feat/improve-objc-header-generator/multithread-package-framents
branch
from
1cf97df to
e5d5e4d
Compare
3 participants
After looking at the profile I found some sections that can be process in parallel to improve framework generation speeds
KT-82436