ankidroid / Anki-Android

AnkiDroid: Anki flashcards on Android. Your secret trick to achieve superhuman information retention.
GNU General Public License v3.0
8.71k stars 2.24k forks source link

Java Heap Space Error #10242

Closed codingtosh closed 2 years ago

codingtosh commented 2 years ago
Reproduction Steps
  1. Convert a java file to kotlin (in this case, PreferenceExtensions.java)
  2. Commit > Extra Commit for .java > .kt renames

EDIT: Amending conversion commits also fails. Screen capture below for reference.

Expected Result

1 File Committed

Actual Result

0 file committed, 1 file failed to commit: [Kotlin Migration] PreferenceExtensions package: com.ichi2.preferences Running Ktlint over these files: AnkiDroid/src/main/java/com/ichi2/preferences/PreferenceExtensions.kt FAILURE: Build failed with an exception. * What went wrong: Execution failed for task ':AnkiDroid:runKtlintFormatOverMainSourceSet'. > A failure occurred while executing org.jlleitschuh.gradle.ktlint.worker.KtLintWorkAction > Java heap space * Try: > Run with --stacktrace option to get the stack trace. > Run with --info or --debug option to get more log output. > Run with --scan to get full insights. * Get more help at https://help.gradle.org BUILD FAILED in 4s Cleaning the project and retrying. > Configure project :AnkiDroid WARNING:The option setting 'android.jetifier.ignorelist=bcprov' is experimental. > Task :AnkiDroid:clean > Task :api:clean > Task :lint-rules:clean Deprecated Gradle features were used in this build, making it incompatible with Gradle 8.0. You can use '--warning-mode all' to show the individual deprecation warnings and determine if they come from your own scripts or plugins. See https://docs.gradle.org/7.3.3/userguide/command_line_interface.html#sec:command_line_warnings BUILD SUCCESSFUL in 1s 3 actionable tasks: 3 executed FAILURE: Build failed with an exception. * What went wrong: Execution failed for task ':AnkiDroid:runKtlintFormatOverMainSourceSet'. > A failure occurred while executing org.jlleitschuh.gradle.ktlint.worker.KtLintWorkAction > Java heap space * Try: > Run with --stacktrace option to get the stack trace. > Run with --info or --debug option to get more log output. > Run with --scan to get full insights. * Get more help at https://help.gradle.org BUILD FAILED in 4s

Research

What was tried: Unchecking git hooks in the commit window allows for proper execution of commits. Problem lies with commit hooks, specifically lint check. (to circumvent the issue, cleaning the project, restarting the IDE and rebuilding worked for the most part)

  1. Settings > System Settings > Memory settings > Kotlin daemon max heap size (currently 3072 MB) -> change to 4096MB. 1.1 Output: No change in commit failure
  2. ktlint { version = "0.43.0" } in build.gradle 2.1. Output: Commit execution not getting stopped
  3. tasks.withType { workerMaxHeapSize.set("4096M") } in gradle.properties 3.1. Output: Worked. Commit not failing anymore. (EDIT* Not working on amend failures)

NOTE: Once the commit were executed without fail , the failure could not be reproduced any longer(despite not changing the build.gradle even in different branches and even resetting the same branch to the point of failure from the reflog).

EDIT: When trying to amend https://github.com/ankidroid/Anki-Android/pull/10249, same commit failure occurs again. HeapSpaceError_commitFailure

codingtosh commented 2 years ago

as per the discussion with @mikehardy , this is due to the long lived gradle worker processes, and a build affects future builds so this off and on failing behaviour is what is expected when the resource limit is reached.

As for now, increasing the heap size of the exact Worker process that was failing has fixed it for amends as well, same behaviour as earlier. Fix once, works everywhere then on even without the changes from the fix. This is what I had added in the gradle.properties:

tasks.withType<org.jlleitschuh.gradle.ktlint.worker.KtLintWorkAction> {
workerMaxHeapSize.set("4096M")
}

But this was just a lazy hack to remove blockades in the migration PRs, more investigation here is required as mentioned by @mikehardy :

Reproducing the problem from a cold start (no old gradle processes running) to failure (possibly across multiple builds / lint runs after cold start) will be important to truly know what limit we are hitting, what exactly triggers it, and how far we need to move

mikehardy commented 2 years ago

I'm okay with a PR that implements the initial lazy hack to unblock things, then this issue can be to determine what the difference really needs to be vs whatever the default is

mikehardy commented 2 years ago
codingtosh commented 2 years ago

@mikehardy Thanks a lot for the references, went through them and it appears to be an issue with memory leak in ktlint itself. Going to send in a PR with our option to increase the heapspace, so just to confirm:

  1. Raising the heap size to 4096M as opposed to the default 256M. (want to make sure it is not excessive?)
  2. Add the changes within build.gradle and not gradle.properties? Could you please point where exactly in build.gradle I should add this?
mikehardy commented 2 years ago

I think it can go anywhere in AnkiDroid/build.gradle, and there should be some sort of logging you can turn on to verify it (with gradlew --stop used between runs to get a clean setup I think)

I'm not sure how much heap is "reasonable", I have 64G of ram in my main workstation so I'm insensitive to it. Others may not be, use your best judgement

codingtosh commented 2 years ago

@mikehardy I'm getting this wherever I try to add it: Could not get unknown property 'withType' for task set of type org.gradle.api.internal.tasks.DefaultTaskContainer

Should I add it to gradle.properties? That worked for me earlier.

mikehardy commented 2 years ago

Define 'worked'. I submit you can put nearly anything in properties and it won't fail but it will have no effect. I put it in build.gradle and it definitely did not throw that error. I'd keep playing with it

codingtosh commented 2 years ago

Hi @mikehardy ,

Define 'worked'. I submit you can put nearly anything in properties and it won't fail but it will have no effect.

Earlier when the commits were failing, I had made the changes in the gradle.properties and the commits were no longer failing from then on. But I take your point, it was probably just because of me restarting/rebuilding and cleaning instead of making the changes in gradle.properties.

I put it in build.gradle and it definitely did not throw that error. I'd keep playing with it

So I was blocked on this, tried to put

tasks.withType<org.jlleitschuh.gradle.ktlint.worker.KtLintWorkAction> {
workerMaxHeapSize.set("2048M")
}

in pretty much all the codeblocks across all the build.gradle files one by one, but to no avail. Turns out, the code I was trying to add above is in Kotlin DSL, but all our gradle files are in groovy DSL. So after changing the syntax, the builds are successful without the error.

tasks.withType(org.jlleitschuh.gradle.ktlint.worker.KtLintWorkAction) {
    workerMaxHeapSize = "2048m"
}

Though I still haven't been able to find a way to verify how to log the maximum heap size a particular worker process in the builds.

  1. Tried ./gradlew --scan but there was no such info on there.
  2. ./gradlew --status to find the pid and then, ps -ef | grep (pid) | grep Xmx , but this only shows the total Java heap space, not the task specific one.

Could you point me to the right direction on how to log it? Meanwhile, opening the PR for the CI.

mikehardy commented 2 years ago

Ah bummer, well that's a shame they don't log out what they have. I suppose you could turn on verbosegc on all spawned VMs but that's really vague as a suggestion, because how do you do that? I have no idea sadly. I'm willing to accept the build.gradle change based on my personal experience and reading of the related issues though - I commented on the PR already - thanks for chasing this

david-allison commented 2 years ago

Was this closed by the PR, or are there still open issues here?

mikehardy commented 2 years ago

A good question. The PR means it should not be happening to people, however I think the underlying issue is still open. Appears to be a bug in prettier ktlint, which is a transitive of gradle ktlint plugin, which is our dependency.

Appears 0.45.0 of prettier ktlint fixed it, they are on 0.45.1 as of this typing Unfortunately the gradle ktlint plugin has not upgraded? Appears to be on 0.42? https://github.com/JLLeitschuh/ktlint-gradle/blob/ddd465e28d77b879384886e1eef5666ebe518b4d/plugin/gradle/libs.versions.toml#L3

Quick look there indicates there may be an override mech: https://github.com/JLLeitschuh/ktlint-gradle/pull/549#issuecomment-983519014

Yep: https://github.com/JLLeitschuh/ktlint-gradle#testing-ktlint-snapshots

I'll post a PR with a change to 0.45.1 to see if it works, if it does then I'll be reasonably satisfied this is actually closed.

codingtosh commented 2 years ago

@mikehardy Thanks for the detailed explanation and the quick patch.