search

quickjs-ng / quickjs

QuickJS, the Next Generation: a mighty JavaScript engine
https://quickjs-ng.github.io/quickjs/
MIT License
972 stars 79 forks source link

Fix handling of memory limit #385

Closed saghul closed 4 months ago

saghul commented 5 months ago

Default to 0, which is "disabled", just like the stack limit.

saghul commented 5 months ago

I'll give it another shot.

saghul commented 5 months ago

@chqrlie PTAL and let me know what you think!

chqrlie commented 5 months ago

@saghul : as you changed the default unit, you should update the v8.js test file:

-                   "--stack-size", `${flags["--stack-size"]*1024}`,
+                   "--stack-size", `${flags["--stack-size"]}`,

This will fix the ci / linux (Release) test failure.

saghul commented 5 months ago

Good one! I got a bit busy and hadn't dug deep enough, thanks for the pointer!

saghul commented 4 months ago

@chqrlie Updated, PTAL!

chqrlie commented 4 months ago

@chqrlie Updated, PTAL!

Hi @saghul ! You only addressed the v8 issue. Any reason not to simplify parse_limit?

saghul commented 4 months ago

@chqrlie Updated, PTAL!

Hi @saghul !

You only addressed the v8 issue. Any reason not to simplify parse_limit?

I'm not seeing any comments on that function on GH. I wonder if they got lost with the force push.

Can you please repeat? :-)

chqrlie commented 4 months ago

@chqrlie Updated, PTAL!

Hi @saghul ! You only addressed the v8 issue. Any reason not to simplify parse_limit?

I'm not seeing any comments on that function on GH. I wonder if they got lost with the force push.

Can you please repeat? :-)

Sorry, I had not published my review...

saghul commented 4 months ago

@chqrlie Thanks a lot for the remark on strtod I wasn't aware of it! Updated, PTAL!