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

Performance javascript grammar #409

Closed
juanfranblanco opened this issue Apr 7, 2023 · 7 comments
Closed

Performance javascript grammar #409

juanfranblanco opened this issue Apr 7, 2023 · 7 comments

Comments

@juanfranblanco
Copy link

Hi,
I have been using a variant of the javascript grammar for a while, but I have found a performance issue with multiple calls in a single statement.

The example function is:

    function test(
         x,
         y,
         z,
         m,
         a,
         b,
         c
    )  {
        
            var monkey = call(
                call2(
                    object1.call3(
                        "xxxxx",
                        call4(),
                        call5(
                            object2.call6(
                                call7(
                                    "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
                                ),
                                x,
                                y,
                                z,
                                m++,
                                s
                            )
                        )
                    )
                ),
                a,
                b,
                c
            );
    }

as you can see this is due to many inner calls, up to call6 the performance is around 1 second the it might be more than 2 seconds..

Do you have any ideas on how to improve this?

@hildjj
Copy link
Contributor

hildjj commented Apr 7, 2023

Can you isolate this to a small grammar input that shows the same behavior? These sorts of things are sometimes ReDoS-style issues.

@juanfranblanco
Copy link
Author

Yes that might be the case although is rather complex to reduce the grammar we have CallExpressions https://github.com/peggyjs/peggy/blob/main/examples/javascript.pegjs#L631MethodExpressions etc. Maybe creating a MultiCallExpression might be able to help.

@hildjj
Copy link
Contributor

hildjj commented Apr 7, 2023

If you turn on tracing, you might see the problem, with the same piece of input being parsed a lot.

@juanfranblanco
Copy link
Author

juanfranblanco commented Apr 12, 2023

@hildjj Fixed it on my end, but if anyone encounters this adding caching (or the lack of) was one of the culprits, it might have been on the migration from peg that the setting was removed. Thank you very much.

@hildjj
Copy link
Contributor

hildjj commented Apr 12, 2023

Did it work better with caching on, or with caching off? We might not have quite enough thought given to how caching interacts with the newer features.

@juanfranblanco
Copy link
Author

Caching on, it made a huge difference.

@hildjj
Copy link
Contributor

hildjj commented Apr 13, 2023

Nod. You should still take a look at your grammar for ReDoS-style issues then. Places where you have (a*)+ for example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants