-
Notifications
You must be signed in to change notification settings - Fork 66
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
v0.4.5-beta.1: dmd runs out of memory #272
Comments
To see what the memory consumption is using a 64 bit compiler, I tested with ldc 1.20.0: Using v0.4.4, compilation takes a maximum of 2 GB of memory. With v0.4.5-beta.1 the memory consumption maxes out at 10.7 GB, then successfully completes compilation. That's a solid 5x in memory consumption during compilation, which also takes a lot longer. Note that this has nothing to do with parsing or parser generation, the parser as a module is already generated, and we're not yet applying the parser. I guess something has changed inside the the templates in |
I had expected a little increase here and there, but this is rather much. I'll need to have another look at it. |
To reproduce: |
The Extended Pascal grammar is special in that it has left-recursion in it. But that mainly causes complications in parsing, not compilation, I think. |
Does this still occur if you use command-line argument |
The Tested on Windows by adding
to |
Looking into it now. |
The resulting binary is 10 times bigger, which suggests this is a template code bloat issue. |
There has been talk about removing one level of templating #190 (comment). I don't know if that would be significant here. Please also unit test, I don't know if #276 (comment) is related. |
Yes, I can confirm the template bloat. 0.4.5-beta.1 has 93405 symbols whereas 0.4.4 has only 23032 Will figure out the largest offenders and get the count down, that should improve speed and memory consumption as well. |
I just did, and |
I remember running them and them succeeding. Regardless, I am looking into the mem consumption right now anyway, so I'll fix the tests afterwards as well. |
I setup appveyor to run 'dub test' on Windows :
|
I have got the symbols down to 23k, so that should no longer trigger. About to make PR. |
@veelo Could do you remove from .travis.yml, this lines ? :
I forgot to remove, and without removing it, it would only be launched by commits/merges on master branch. |
Fix #272 by moving template code around
The unittest failure is a small one, about to fix. |
Essentially we never need to keep the discard nodes
Fixes unittests failure introduced by #272
Thanks for your work on this! Unfortunately, it appears not to be enough for x86: https://ci.appveyor.com/project/PhilippeSigaud/pegged/build/job/nollc0299oow3crs Reopening this for now. |
Looks that only happens with dmd 2.090 . With 2.091 works fine . Could be really a bug on dmd 2.0.90 ?
appveyor output:
|
2.091 started shipping dmd in 64 bit version, which is capable of allocating more memory. On 32 bit Windows it obviously still runs 32 bit dmd, and also on low available memory this would be a problem. It could be that we just crossed the boundary with a moderate increase, and then so be it (that's why I removed the "bug" label), but if possible it would be very nice to not grow the memory footprint too much. |
I try something... I go to v0.4.4 and try to build extended_pascal with this result :
|
WTF! I try a second time (with removing parse_ep and source/eppparse.d files) then doesn't fails!
|
Ok ... so I did a runs with v0.4.4 v0.4.5-beta.1 and commit 21994c1 v0.4.4First run : v0.4.5-beta.1First run : Commit 21994c1 (just before merging #269. )First run : So, yes... something went very wrong with the merge #269 |
Please try after #280 as well, as I fixed the memory issue there. |
Yep! It's fixed : First run : So, now discover why a test crash with dmd on windows |
Great! |
This is most likely due to dlang/dub#1474. |
Testing the latest commit d5b3dd9 on Windows 10, dmd v2.091.0 32-bit with
monitoring with Process Explorer, the Peak Working Set is still quite high: >1.947.000K. This is very close to the address space limit of 2GB for a 32-bit user-mode process on Windows. I suspect that dmd 2.090.1 requires a bit more, making it cross that limit. Testing the same with 64 bit dmd:
the Peak Working Set is higher still: >3.040.000K. Testing af79adb, just before merging #272, dmd 2.091.0 32-bit requires >1.314.000K. Still a lot, but comfortably under the limit. dmd 2.090.1 32-bit indeed requires slightly more memory for the same commit: >1.458.000K. Keeping track of the longest match throughout the parse obviously requires more memory. The question I think is this: is an increase of 48% reasonable to expect? And if yes, is that generally affordable? Note that this is only of value for a failed parse, so that if the answer is no, it may still make sense to put this behind an option or something. I think I'll tag a new beta release and then test it on my large grammar with large inputs, to see what the memory usage becomes. |
Sorry, I was confusing the compilation with the parsing. The parsing obviously requires more memory, but the failed allocation is during compilation. Why is it that my numbers are so much higher than those measured by @Zardoz89? |
Could be a issue of DMD on windows ? I asked on DLang forums, and looks that the least versions of DMD have some weird issues : |
@veelo and @Zardoz89 Thanks for looking into this so much and spending the effort you do to get my changes in. My changes in #269 don't allocate much more memory, but just retain more objects. Normally when parsing complete ParseTree's would have been rejected and collected by the GC, but now are retained for better diagnostics. There is the general solution of having pegged use less memory (e.g. by reducing the size of the ParseTree struct, or replace the ~ used for array concatenation with Appenders). Another one might be to discard the failed longest match when they no longer are the longest. This should allow the GC to free more memory while pegged is parsing. At the cost of a little cpu to revisit earlier nodes. |
It keeps confusing me as well, but since it is dmd that runs out of memory during compilation of the unittests, before it runs them, I am not sure whether it is the additional memory needed for retaining the longest match that is the problem. It could be, I guess, if it involves some CT parsing. I am almost tempted to disable all unittests but one, and switch them on individually to see which one is causing the problem. But I cannot devote that kind of time to this at the moment... |
https://forum.dlang.org/post/[email protected]
So this explains why apparently only fails with 2.090.1 on Appveyor . So, now why on Windows dmd ram usage is too big ? On linux this isn't happening. https://travis-ci.com/github/PhilippeSigaud/Pegged/jobs/313698926
|
I am facing a much more severe issue, where compiling |
What version of pegged do you use? |
Seems like we really need Alternatively we could merge newCTFE and have it hidden behind a Have anybody tried using newCTFE with Pegged and measured any reductions in time and memory reduction during generation of parser from peg-grammar? I managed to at least merge newCTFE with todays dmd master, which seems promising. |
This one could also be template related.
I need to measure.
…On Tue, Aug 4, 2020, 12:30 PM Per Nordlöw ***@***.***> wrote:
Seems like we really need newCTFE. I wonder if @UplinkCoder
<https://github.com/UplinkCoder> could maintain a feature newCTFE-branch
of dmd until it, hopefully, gets merged some day.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#272 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAVWSCFZKIHUJEMUCPEINTDR67PNNANCNFSM4K4FFXPQ>
.
|
Great. Can you merge master into newCTFE and fix compilation errors while you are at it? |
Using v0.4.4, dmd used to consume just over 1 GB of memory compiling my project. With v0.4.5-beta.1, I can see dmd memory consumption rise to over 1.8 GB, after which dmd fails with a "Memory allocation failed" exception. Same result for dmd 2.089.0 as for dmd 2.090.1.
My parser is generated as a module, not at compile time.
This problem was likely introduced by #269.
The text was updated successfully, but these errors were encountered: