-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
support running inefficient programs in the vm #13135
Conversation
What can I say? I'm not good at it and I have a lot of friends that write even less efficient software.
Why not just add |
Can you give us a reasonable example where this limit really hurts us? |
No, but it hurts me personally:
Can you give me some painful examples where the limit helped? Should I be looking into my performance problem? |
I have also run into this limit and had done the change locally (except as below):
const nimVMMaxLoopIterations {.intdefine.} = 10_000_000
const MaxLoopIterations = nimVMMaxLoopIterations so that user can override, and so that we respect the nim prefix convention (pending nim-lang/RFCs#181)
and this: tests/vm/tmaxloopiterations.nim |
To be clear, I'm just trying to parse some JSON. Sure, it's not tiny, but under 100k lines, or about 4mb. I don't think anyone should have to pass an option to the compiler if they want to consume a few hundred megs of data at compile-time. Maybe I'm alone in this view, however. |
@disruptek the point is there is no "one true limit" (except unlimited but that would prevent catching certain bugs where VM runs in infinite loops for eg); what if your use case happens to be just above the new limit you put? I did run into the existing limit (for a different scenario) and I welcome a PR that looks like #13135 (comment) (and ok if you change the default to something higher as part of the PR in addition to the intdefine) |
I've already made a PR to raise the limit for my use-case. If your use-case happens to be just above the new limit that I put, well, I also welcome a PR that looks like #13135 (comment). But, to my eye, the fact that this PR is not that PR is not sufficient reason to reject it. |
@disruptek I think #13233 is a better approach; it doesn't require recompiling nim to change the default max number of loops |
What can I say? I'm not good at it and I have a lot of friends that write software even less efficient.