Uploaded image for project: 'CouchDB'
  1. CouchDB
  2. COUCHDB-3245

couchjs -S option doesn't have any effect

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: None
    • Component/s: None
    • Labels:
      None

      Description

      currently -S option of couchjs sets stack chunk size for js contexts

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

      Documentation recommends 8K and I have seen cases where it was raised to 1G+ in production!. That doesn't seem right at all and also probably kills performance and eats memory.

      Docs from above say:

      > The stackchunksize parameter does not control the JavaScript stack size. (The JSAPI does not provide a way to adjust the stack depth limit.) Passing a large number for stackchunksize is a mistake. In a DEBUG build, large chunk sizes can degrade performance dramatically. The usual value of 8192 is recommended

      Instead we should be setting the max gc value which is set in the runtime

      JS_NewRuntime(uint32_t maxbytes)

      Experimentally a large maxbytes seems to fix out of memory error caused by large views. I suspect that it works because it stops GC. At some point we probably drops some object, GC collects them and we crash...

        Issue Links

          Activity

          Hide
          vatamane Nick Vatamaniuc added a comment -

          Interesting! Looks like it was fixed before: https://issues.apache.org/jira/browse/COUCHDB-1792

          Show
          vatamane Nick Vatamaniuc added a comment - Interesting! Looks like it was fixed before: https://issues.apache.org/jira/browse/COUCHDB-1792
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user nickva opened a pull request:

          https://github.com/apache/couchdb-couch/pull/216

          Make couchjs -S option take effect

          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

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/cloudant/couchdb-couch couchdb-3245

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/couchdb-couch/pull/216.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #216


          commit 2bca9625aa6769b55520ebe796c5ccb5a55d3091
          Author: Nick Vatamaniuc <vatamane@apache.org>
          Date: 2016-11-30T05:37:12Z

          Make couchjs -S option take effect

          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


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user nickva opened a pull request: https://github.com/apache/couchdb-couch/pull/216 Make couchjs -S option take effect 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 You can merge this pull request into a Git repository by running: $ git pull https://github.com/cloudant/couchdb-couch couchdb-3245 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/couchdb-couch/pull/216.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #216 commit 2bca9625aa6769b55520ebe796c5ccb5a55d3091 Author: Nick Vatamaniuc <vatamane@apache.org> Date: 2016-11-30T05:37:12Z Make couchjs -S option take effect 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
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 1659fda5dd1808f55946a637fc26c73913b57e96 in couchdb-couch's branch refs/heads/master from Nick Vatamaniuc
          [ https://git-wip-us.apache.org/repos/asf?p=couchdb-couch.git;h=1659fda ]

          Make couchjs -S option take effect

          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:

          https://github.com/apache/couchdb-couch/commit/62dafe81e13d7f8e27e95057e65f76d534aa2313

          by @tilgovi

          Reference:

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

          COUCHDB-3245

          Show
          jira-bot ASF subversion and git services added a comment - Commit 1659fda5dd1808f55946a637fc26c73913b57e96 in couchdb-couch's branch refs/heads/master from Nick Vatamaniuc [ https://git-wip-us.apache.org/repos/asf?p=couchdb-couch.git;h=1659fda ] Make couchjs -S option take effect 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: https://github.com/apache/couchdb-couch/commit/62dafe81e13d7f8e27e95057e65f76d534aa2313 by @tilgovi Reference: https://developer.mozilla.org/en-US/docs/Mozilla/Projects/SpiderMonkey/JSAPI_reference/JS_NewContext COUCHDB-3245
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/couchdb-couch/pull/216

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/couchdb-couch/pull/216
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 14157e3ee03f1f0c6ea5580a52b9de3b93bc149c in couchdb's branch refs/heads/master from Nick Vatamaniuc
          [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=14157e3 ]

          Bump couch dependency. Fixes couchjs -S option not taking effect

          Closes COUCHDB-3245

          Show
          jira-bot ASF subversion and git services added a comment - Commit 14157e3ee03f1f0c6ea5580a52b9de3b93bc149c in couchdb's branch refs/heads/master from Nick Vatamaniuc [ https://git-wip-us.apache.org/repos/asf?p=couchdb.git;h=14157e3 ] Bump couch dependency. Fixes couchjs -S option not taking effect Closes COUCHDB-3245
          Hide
          gdelfino Gustavo Delfino added a comment -

          The last comment says implies that this issue is solved, but this ticket is still open. Is this indeed solved for the next CouchDB release? How about the Windows CouchDB 2.0.0.2 released just days ago?

          Show
          gdelfino Gustavo Delfino added a comment - The last comment says implies that this issue is solved, but this ticket is still open. Is this indeed solved for the next CouchDB release? How about the Windows CouchDB 2.0.0.2 released just days ago?
          Hide
          vatamane Nick Vatamaniuc added a comment -

          @gdelfino the issues was fixed just forgot to close the ticket.

          Show
          vatamane Nick Vatamaniuc added a comment - @gdelfino the issues was fixed just forgot to close the ticket.

            People

            • Assignee:
              Unassigned
              Reporter:
              vatamane Nick Vatamaniuc
            • Votes:
              1 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development