Skip to content
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

recursive implementation of the tuple is very inefficient #3307

Open
volodya-lombrozo opened this issue Jul 30, 2024 · 13 comments
Open

recursive implementation of the tuple is very inefficient #3307

volodya-lombrozo opened this issue Jul 30, 2024 · 13 comments
Assignees
Labels

Comments

@volodya-lombrozo
Copy link
Member

I am facing a problem with printing PHI expressions. I encountered the following exception when I ran eo-maven-plugin:0.39.0:xmir-to-phi:

[ERROR] #fatalError(): Too many nested function calls. May be due to infinite recursion; SystemID: file:///org/eolang/parser/set-locators.xsl; Line#: 43; Column#: 11

As you can see, the set-locators.xsl transformation uses too many nested function calls. This problem usually happens on Windows OS, and it's flaky. I have attached the full log below.

build.log

Copy link

@volodya-lombrozo thanks for the report, here is a feedback:

Problems

  • Lack of Reproducibility: The report does not adequately detail the process to recreate the error. It mentions a command that was executed but omits essential information such as the environment, software version, and specific steps leading to the error.
  • Unclear Error Description: The report fails to clarify the meaning of 'Too many nested function calls' in this scenario, potentially causing confusion for developers unfamiliar with this error or system.
  • No Expected vs. Actual Results: The report neglects to specify the anticipated results when executing the command, nor does it describe how the actual outcomes vary from these expectations.

I would recommend including expected versus actual results when executing the command, to identify where the discrepancy lies.

Please fix the bug report in order it to get resolved faster.
Analyzed with gpt-4

@volodya-lombrozo
Copy link
Member Author

volodya-lombrozo commented Jul 30, 2024

@yegor256
Copy link
Member

@maxonfjvipon please, help us here

@volodya-lombrozo
Copy link
Member Author

Blocks objectionary/jeo-maven-plugin#645

@maxonfjvipon
Copy link
Member

@volodya-lombrozo @yegor256 in the provided log processed XMIR is printed and it has 379 nested tuple objects. I think it's too much for us:

  1. it's unreadable
  2. we can't deal with it at the level of phi because it would be also unreadable
  3. it causes such flaky tests

(Stars on line 314, ends on line 690)
image
image

I see two scenarios:

  1. we're finding a way to configure XSLT engine (if it exists) and setting some parameter which is responsible for max nested calls. Transformations will work, it'll be still unreadable and may cause some other unknown problems in the future
  2. we're making an exception and changing the structure of XMIR - instead of tuple we're using some mysterious atom which accepts unlimited amount of parameters. We'll use the atom only for jeo, opeo. So instead of:
tuple
  tuple
    tuple
      tuple
        tuple
          ...
             tuple
               tuple.empty
               1
             2
           ...
         300
       301
     302
   303

we'll get:

array
  1
  2
  3
  4
  ...
  302
  303

Since here we use EO and XMIR as IR - we don't need to run it, which means we can do it. The code's much more readable, does not contain so many nested object and should not cause unpredictable troubles related to large nesting.

WDYT?

@volodya-lombrozo
Copy link
Member Author

@maxonfjvipon To be honest, I like the both suggestions. However, adding "mysterious" atoms might cause some problems in the future as well. I would suggest to try the first point first, then if we don't find anything satisfiable (or it will take significant time,) we can just go by the second path. What do you think?
The first option seems a bit faster to solve. And I don't think we will have many of such long methods with ~> 300 instructions.

@volodya-lombrozo
Copy link
Member Author

@maxonfjvipon Friendly reminder

@maxonfjvipon
Copy link
Member

@volodya-lombrozo here I found the info that such behaviour is related to insufficient stack size (which is 256M now).
Also we can try to optimize XSLs so they use tail recursion instead of head one.
So try to increase stack size for now via command line argument, it should help for now, and I'll check what I can do with tail recursion

@volodya-lombrozo
Copy link
Member Author

@maxonfjvipon Thanks. I will try to increase stack size for now.

@yegor256
Copy link
Member

@volodya-lombrozo I believe, it's fixed?

@volodya-lombrozo
Copy link
Member Author

@yegor256 How? The problem is still here, as far as I understand. @maxonfjvipon Please, correct me, if I'm wrong. If we have a tuple with many inner tuples inside we have a stackoverflow error. For now, I fixed it by increasing the stack size, but on your side there are no changes. Or I'm wrong?

@yegor256
Copy link
Member

@maxonfjvipon we may think about a more optimal implementation of tuple, maybe via tuple3, tuple4, tuple5, and so on atoms?

@maxonfjvipon
Copy link
Member

@yegor256 we definitely should

@yegor256 yegor256 changed the title Too many nested function calls recursive implementation of the tuple is very inefficient Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants