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

Make couchjs -S option take effect #216

Merged
merged 1 commit into from Nov 30, 2016
Merged

Conversation

@nickva
Copy link
Contributor

nickva commented Nov 30, 2016

Previously it was used to set JS context's stack chunk size.

Instead, to be effective it should set max GC size of the runtime.

Stack chunk size was set to the recommended value: 8K

Reference:

https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/JSAPI_reference/JS_NewContext

COUCHDB-3245

@nickva nickva force-pushed the cloudant:couchdb-3245 branch from 2bca962 to 0795fbf Nov 30, 2016
@jaydoane

This comment has been minimized.

Copy link
Contributor

jaydoane commented Nov 30, 2016

+1

Though it's curious how the previous fix https://issues.apache.org/jira/browse/COUCHDB-1792 was apparently never applied. However, it clearly seems to be the right thing to do.

@nickva

This comment has been minimized.

Copy link
Contributor Author

nickva commented Nov 30, 2016

Thanks @jaydoane

I think the fix was applied then switched back during a refactoring?

It is interesting why it works though. I think it probably masking a fundamental and tricky bug with garbage collection. By essentially stopping it from running.

However for practical reasons we saw that at least this brings us back to behavior that we've had before in production.

@jaydoane

This comment has been minimized.

Copy link
Contributor

jaydoane commented Nov 30, 2016

Yes, maybe reverted during a refactor, since all the files that patch applied to (sm170.c, sm180.c, sm185.c) no longer exist. I guess that's easy to do with an absence of unit tests ;)

@davisp

This comment has been minimized.

Copy link
Member

davisp commented Nov 30, 2016

+1. But add a reference to Randall's commit that somehow got reverted.

@davisp

This comment has been minimized.

Copy link
Member

davisp commented Nov 30, 2016

This one: 62dafe8

And make sure the rest of that commit is applied as well.

Previously it was used to set JS context's stack chunk size.

Instead, to be effective it should set max GC size of the runtime.

Stack chunk size was set to the recommended value: 8K

This brings back an accidentally reverted commit:

62dafe8

by @tilgovi

Reference:

https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/JSAPI_reference/JS_NewContext

COUCHDB-3245
@nickva nickva force-pushed the cloudant:couchdb-3245 branch from 0795fbf to 1659fda Nov 30, 2016
@asfgit asfgit merged commit 1659fda into apache:master Nov 30, 2016
1 check was pending
1 check was pending
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.