Fix for GC on windows #1235
Fix for GC on windows #1235
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1235 +/- ##
==========================================
+ Coverage 87.16% 87.20% +0.03%
==========================================
Files 113 113
Lines 10287 10294 +7
Branches 4045 4050 +5
==========================================
+ Hits 8967 8977 +10
+ Misses 727 726 -1
+ Partials 593 591 -2 ☔ View full report in Codecov by Sentry. |
|
|
||
| // printf("consider gc %d (%ld, %ld, %ld, %ld) %ld\n", run_gc, | ||
| // current, low_water, high_water, limit, limit - pred); | ||
| #if 0 |
There was a problem hiding this comment.
Did you forget to commit something? This is unconditional true, I think it should be activated upon instrumentation, right?
It'd also be nice to use a consistent formatting (tabs vs spaces)
There was a problem hiding this comment.
The instrumentation is to barf out all of the python gc calls and all the java calls. I am not sure anyone is likely to want that unless they are trying to debug the process. So I currently have it conditionally compiled as false all the time.
I will try to fix up the tabs. The problem is that some files we have are currently tab spaced and others are spaced only and I have to keep switching the settings on vim based on which file I am editing.
There was a problem hiding this comment.
I've found it rather painful as well while working on extension support. It got to the point where I've said to hell with it and reformatted entire files and decided to just deal with fixing it later.
There was a problem hiding this comment.
We should come up with some codestyle probably. Did you choose spaces over tabs or vice versa @astrelsky?
There was a problem hiding this comment.
We should come up with some codestyle probably. Did you choose spaces over tabs or vice versa @astrelsky?
My ide settings were set to use this formatter so it only took me two clicks to reformat a file. https://github.com/NationalSecurityAgency/ghidra/blob/master/eclipse/GhidraEclipseFormatter.xml
I learned java from working with Ghidra so I lean towards their code style with minor divergence. In many ways it directly conflicts with the styles used in jpype. I've become very nitpicky about formatting.
To actually answer your question, I used tabs (width of 4 spaces). At the end of the day I'll follow whatever code style you set up. I was aware I was creating extra work for myself by applying the formatter when I did it.
I don't think I should have any say here. As long as it's consistent I'll follow it 😂
There was a problem hiding this comment.
It wouldn't be much of an effort for them to make an option like my "python setup.py build_ext --makefile" which just takes all the instructions they are running on a system and dumping it out in makefile format. I would never have made it this far in the project without it. Unfortunately with the motion towards a more opaque process it is getting harder to accomplish such development.
fwiw cmake does a pretty decent job with python extensions. The only thing that was lacking when I set it up was free threaded python support. It might be able to generate the netbeans project for you too.
There was a problem hiding this comment.
I'd be in favor of tools which are independent of the used IDE. Otherwise everybody would be forced to use the same IDE, right?
There was a problem hiding this comment.
I'd be in favor of tools which are independent of the used IDE. Otherwise everybody would be forced to use the same IDE, right?
I'm always fighting someone to make something os/ide independent, even at work. Everyone should be able to use what they're most comfortable and productive with.
There was a problem hiding this comment.
I just want a beautifier to run from command line when we check in. So long as the style is close enough that anyone's personal IDE doesn't go insane trying process it then all is good. Double points if the beautifier is a python module so that we can just pull it in with requirements rather than installing another tool. I distributed the Netbeans IDE files in the project directory, but there is no expectation that anyone needs to use them.
(And yeah I understand IDE/build tool wars. I have developed a code base for 20 years and every new hire wants to use their IDE and a new build tool when they are going to at most touch 1% of my code base. Dropping my productivity a few precent to deal with their shiny new tool pretty much negates their value.,)
There was a problem hiding this comment.
Right, so lets run pre-commit on PRs, so everybody can do whatever they love offline. When the code goes online, we enforce the formatting (this can be done automatically, but would involve rebasing to origin after a CI run).
It is also called evolution when the young ones wants to introduce state of the art stuff. I was also young one day and my bosses hated me for introducing SVN over CVS =) Sooo complicated. Several years later we had the same thingy with Git. Now everybody uses it. So after all I wouldn't say it's a bad thing to modernize once in a while.
The productivity drop is preliminary and would probably benefit in the future.
native/common/jp_gc.cpp
Outdated
| run_gc = 2; | ||
| limit = high_water + (high_water>>3) + 8 * (current - last); |
There was a problem hiding this comment.
can you please explain this heuristic? What does it aim for?
There was a problem hiding this comment.
The current logic is that if Python memory was rising above the high water (or predicted to) then it would trigger then java gc. When the python gc is complete we were supposed to reset the high water so that we won't run the java gc again for a while.
In Windows, it appears that rather than stop the world Python is still running. So what is happening is that in the example sent we triggered the java gc over and over and over. To prevent that I have to raise the limit above the waves so that it won't trigger unless garbage is actually piling up. Here is a graph of before and after.
|
@Thrameos Do you want this to be included in 1.5.1? |
commit d837337 Merge: 1a56a3b c474ee8 Author: Martin K. Scherer <[email protected]> Date: Mon Nov 18 08:34:35 2024 +0100 Merge pull request #1235 from Thrameos/win_gc Fix for GC on windows commit c474ee8 Author: Karl Nelson <[email protected]> Date: Sun Nov 17 20:25:19 2024 -0800 Fix tab/spaces commit 3d2577a Author: Karl Nelson <[email protected]> Date: Sat Nov 16 18:15:44 2024 -0800 Fix for GC on windows
|
I think it should go in. It was a last minute fix for a critical speed bug. It may be a while before we have another release. |
|
I wish it was true on the productivity. But now a days it seems like people are being spoon fed general purpose tools that they use for every class. My old netbeans with ant does everything their tools do plus a huge amount more. If they wreck my performance and force me to waste hour after hour dealing with groovy/maven/random tool which they actually don’t even know how to use themselves, then it is a loss. Better to retool them to use tools that work. If they can use another IDE without busting up my 60k lines of code I am maintaining then great, if not then use my tools.
|
|
It seems you're kind angry about that. Wasting time on something which does not work or boost productivity is indeed a huge loss. Any how, please stop cross posting issues with your greatly working email client :P just being sarcastic |
Could be worse, I've seen people use make for Java 😂. Does it work, yes, but is it the best tool for the job, no not really. |
The issue appears to be with the Python bookkeeping. When the Python memory is growing faster than the Java GC can execute the Java GC gets triggered many times resulting in a serious slow down. I upped the limit after a forced Java trigger. I also added some instrumentation to make diagnosis easier. It is not a 100% but does address the issue.
Fixes #1197