Out of memory error during installation

panther2 commented 4 years ago

outof

Installation breaks at that point. Never had that before...

RoiEXLab commented 4 years ago

@DanVanAtta I'm starting to agree with you that the recent OOM errors are related to pack200 for some reason. I had a look at the actual source mentioned by the stacktrace @panther2 thankfully provided, it doesn't seem to be some sort of "bug" either, just some "normal" buffer re-allocation: https://github.com/AdoptOpenJDK/openjdk-jdk11/blob/19fb8f93c59dfd791f62d41f332db9e306bc1422/src/java.base/share/classes/com/sun/java/util/jar/pack/NativeUnpack.java#L282

Given that Pack200 no longer exists in JDK 14 (which is about to be released in a couple of weeks) we should really consider shrinking down our shipped JRE in order to save disk space using a different approach 🤔

panther2 commented 4 years ago

@RoiEXLab @DanVanAtta

Is that related to https://github.com/triplea-game/triplea/pull/5939? If yes, reading the comment to that PR it seems that 3GB might be just enough. So maybe it could be a workaround to revert this until a better solution can be realized?

I am just asking because I am excluded from testing new pre-releases now.

RoiEXLab commented 4 years ago

@panther2 I'm starting to believe that it's the same problem. You could try launching the installer via command line like this:

.\TripleA_installer.exe -J-Xmx4G

Where you use the real installer name instead od the one I used of course.

panther2 commented 4 years ago

@RoiEXLab Unfortunately the same error occurs.

RoiEXLab commented 4 years ago

Hmm, you could try increasing the number even further, but at this point I'm not sure if this will make a difference. Is this the only release you're having problems with? Or does it apply to other releases as well?

panther2 commented 4 years ago

@RoiEXLab I noticed it for the first time here. I updated from 18027 which installed flawlessly.

DanVanAtta commented 4 years ago

@RoiEXLab my first thought is for us to change sub-project structure so that 'game-headed' no longer contains a launch-able, instead we have 'swing' and 'javafx' that sit on top, thereby creating different artifacts with different dependencies. If we are pretty sure that the pack200 issues are related, then this would be a good step both in terms of architecture, resolving this problem, and reducing installer size of 2.0.

Beyond that, @RoiEXLab can you think of or are considering any other good options that we can get done in the short term?

RoiEXLab commented 4 years ago

Not really. I'd recommend adding some debug information to the build so we can actually verify pack200 is causing the issue when it occurs. If that's confirmed we could try disabling it and check by how much this increases the installer size. In case it's only a couple of megabytes that would be no big deal, in other cases creating a different installer for the JavaFX components would be the best mid-term solution until the UI can be considered a solid replacement.

DanVanAtta commented 4 years ago

How do you mean not really? What debug info can we add and how? AFAIK, oom for cheap is debugged by doing a class histo. If we can look at the installer with a profile while it is running, we might see the problem. Though, even then, it may be unclear if related to pack200

Overall I'm most concerned by this current issue. We also are not sure if it is related to pack200 problems, though there are thousands of warnings about, presumably it is 'a' problem. Creating a second installer for FX seems to be a good move even if it does not solve the installer crash. If it does, we buy time to solve that problem since presumably the fx install would crash.

Perhaps we can at least validate this by lowering heap to get consistent oom, remove fx components and see if we see any problems

On Mon, Feb 24, 2020, 2:59 PM RoiEX notifications@github.com wrote:

Not really. I'd recommend adding some debug information to the build so we can actually verify pack200 is causing the issue when it occurs. If that's confirmed we could try disabling it and check by how much this increases the installer size. In case it's only a couple of megabytes that would be no big deal, in other cases creating a different installer for the JavaFX components would be the best mid-term solution until the UI can be considered a solid replacement.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/triplea-game/triplea/issues/5973?email_source=notifications&email_token=AC6SZOKKGEXZ3N6G476YBBLRERGN7A5CNFSM4KZTJSP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMZ3R7A#issuecomment-590592252, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC6SZOOYBXIKH32PRGINVCLRERGN7ANCNFSM4KZTJSPQ .

RoiEXLab commented 4 years ago

@DanVanAtta Well I did consider shrinking the runtime as short-term solution. After all the jdeps tool makes it rather easy in comparison to figure out the required modules for our code. For some weird reason it had some issues with JavaFX, haven't figured that one out yet, but it works fine on the generated class files from the compiler. So the hard part would be to figure out which of our dependencies do require what kind of modules. If we want to be precise, we should even check if we really need all of guava and maybe are able to drop some module that is only required by some unused part of it for example. So if someone wants to spend a weekend configuring install4j correctly and ideally add a CI step that either checks, or automatically generates the required modules list, then that would be a nice improvement. However I currently don't have this time, maybe in a month.

DanVanAtta commented 4 years ago

Anything you can suggest that we can do in days? 2.0 is overdue

On Mon, Feb 24, 2020, 11:12 PM RoiEX notifications@github.com wrote:

@DanVanAtta https://github.com/DanVanAtta Well I did consider shrinking the runtime as short-term solution. After all the jdeps tool makes it rather easy in comparison to figure out the required modules for our code. For some weird reason it had some issues with JavaFX, haven't figured that one out yet, but it works fine on the generated class files from the compiler. So the hard part would be to figure out which of our dependencies do require what kind of modules. If we want to be precise, we should even check if we really need all of guava and maybe are able to drop some module that is only required by some unused part of it for example. So if someone wants to spend a weekend configuring install4j correctly and ideally add a CI step that either checks, or automatically generates the required modules list, then that would be a nice improvement. However I currently don't have this time, maybe in a month.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/triplea-game/triplea/issues/5973?email_source=notifications&email_token=AC6SZONETEZJXFPGI4724PDRETAGPA5CNFSM4KZTJSP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEM22NGQ#issuecomment-590718618, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC6SZOI4K3LFILRCHD5PHI3RETAGPANCNFSM4KZTJSPQ .

DanVanAtta commented 4 years ago

That is a bit why I suggest the project structure update. Can be done in hours, particularly if we comment out the breakages. There are then some conversations about how to get it working properly, but at least then we have bought ourselves some time.

On Tue, Feb 25, 2020, 9:17 AM Dan Van Atta dhvatta@gmail.com wrote:

Anything you can suggest that we can do in days? 2.0 is overdue

On Mon, Feb 24, 2020, 11:12 PM RoiEX notifications@github.com wrote:

@DanVanAtta https://github.com/DanVanAtta Well I did consider shrinking the runtime as short-term solution. After all the jdeps tool makes it rather easy in comparison to figure out the required modules for our code. For some weird reason it had some issues with JavaFX, haven't figured that one out yet, but it works fine on the generated class files from the compiler. So the hard part would be to figure out which of our dependencies do require what kind of modules. If we want to be precise, we should even check if we really need all of guava and maybe are able to drop some module that is only required by some unused part of it for example. So if someone wants to spend a weekend configuring install4j correctly and ideally add a CI step that either checks, or automatically generates the required modules list, then that would be a nice improvement. However I currently don't have this time, maybe in a month.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/triplea-game/triplea/issues/5973?email_source=notifications&email_token=AC6SZONETEZJXFPGI4724PDRETAGPA5CNFSM4KZTJSP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEM22NGQ#issuecomment-590718618, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC6SZOI4K3LFILRCHD5PHI3RETAGPANCNFSM4KZTJSPQ .

DanVanAtta commented 4 years ago

I need to head to work, so sorry for this being a quickly written note/plan. The month long time frame before we even validate if we have solved the pack200 problem is distressing. Long story short that pushes the 2.0 release to potentially after summer;

one month for validation of pack200. (early april)
Assuming that is good; 4-10 weeks for pre-release testing and fixes (putting us into late may)
then we are in summer, capacity drops, release items won't get picked up until August, things will drag and we are talking 2.0 being released sometime in the fall of 2020.

That kind of timeline I do not think is acceptable at all.

We need some options, please pitch in if there are thoughts on a good plan. My suggestion is we do this:

merge the in-flight javafx PR
create a sub-project for javafx, do not add it to the compilation unit
move all javafx dependencies and classes to that sub-project
validate that we no longer see pack200 warnings and that the OOM has gone away
regain momentum on prerelease/QA testing; I'm beyond frustrated that we lost that momentum and are seemingly in a pattern of wait for the first problem to be fixed before doing anything else (then again, this OOM is blocking all of that anyways, hence why this is such a significant problem)
finish out any fixes, hopefully this all can be done within the next 3 weeks and we can start soft launch

Then:

IMO we need to reconsider how we approach javafx. As a 2.1 or 3.0 target, I'm willing to help on that project and we should get better momentum with multiple people working on it. Though, the strategy I think needs to be discussed, rather than hacking/modifying the game launcher to get it to work with a nicer UI, we may need to first improve the game launch code and properly split the UI concerns. If we do the hacking route, then we'll be left with nice UI but still horribly complex game launching code. I think we need to invert that order of operations so that javafx lands easily. Essentially that means the fun part of doing UI work needs to be postponed a bit for us to first clean up the underlying wiring and launching code. Then, once that is done, top-level packages so far I think have had mixed success (EG: headless-game). I think we'll need an injection framework to be able to sanely create items in a javafx package and have them be injected in the right location in the underlying packages (so something like dagger, guice or spring). Ultimately though this is a strategy conversation that should happen as soon as we are in soft-launch, before then all focus should be on resolving our first lead of the pack200 errors, getting the installer solid and doing loads of QA so we don't burn the community with a bad release.

RoiEXLab commented 4 years ago

Anything you can suggest that we can do in days? 2.0 is overdue

@DanVanAtta Sorry, that's all the ideas I currently have

DanVanAtta commented 4 years ago

@Roi, thoughts on the plan? Best case we buy time and get a project structure improvement, worst case we only get a project structure improvement and removed pack200 warnings as a suspect.

On Tue, Feb 25, 2020, 11:14 AM RoiEX notifications@github.com wrote:

Anything you can suggest that we can do in days? 2.0 is overdue

@DanVanAtta https://github.com/DanVanAtta Sorry, that's all the ideas I currently have

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/triplea-game/triplea/issues/5973?email_source=notifications&email_token=AC6SZOOZ2ZTQK5GXJXBO3A3REVUZTA5CNFSM4KZTJSP2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEM5D6SI#issuecomment-591019849, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC6SZOPN5JZ2GMBW5DWR4JDREVUZTANCNFSM4KZTJSPQ .

RoiEXLab commented 4 years ago

Sounds good, I'll have some spare time tomorrow so I might even be able to get the JavaFX PR "done" tomorrow or Thursday if you want to wait for it. Alternatively I could wait until we executed the plan and submit the rest afterwards

DanVanAtta commented 4 years ago

@RoiEXLab , please mention here if you get a chance to get started on:

create a sub-project for javafx, do not add it to the compilation unit

move all javafx dependencies and classes to that sub-project

To be clear, the above is meant to be quick/fast, the moves would be breaking to the components that are moved, but since they are no longer part of the compilation unit (ie: the sub-project would not be added to settings.gradle), we have time to make fixes.

RoiEXLab commented 4 years ago

I'm currently just waiting for #5917 to be merged, after that it shouldn't be a big deal to create a seperate sub-project. However I don't understand why you don't want this to be a sub-project, if there's no install4j task, it won't get packed into the installer, right?

DanVanAtta commented 4 years ago

@RoiEXLab AFAIK the new sub-project would be something like 'game-headed-javafx' and would depend on 'game-headed'. If all the components can be moved quickly into that project and still compile, it's a bonus. Otherwise the goal is to accomplish that move quickly. My impression is that it would be some work to get such a top-level sub-project to work and is part of the reason we have not already created a separate javafx installer.

triplea-game / triplea

Out of memory error during installation #5973