Skip to content

  • Projects
  • Groups
  • Snippets
  • Help
  • This project
    • Loading...
  • Sign in / Register
gjs
gjs
  • Overview
    • Overview
    • Details
    • Activity
    • Cycle Analytics
  • Repository
    • Repository
    • Files
    • Commits
    • Branches
    • Tags
    • Contributors
    • Graph
    • Compare
    • Charts
  • Registry
  • Issues 87
    • Issues 87
    • List
    • Board
    • Labels
    • Milestones
  • Merge Requests 15
    • Merge Requests 15
  • CI / CD
    • CI / CD
    • Pipelines
    • Jobs
    • Schedules
    • Charts
  • Wiki
    • Wiki
  • Snippets
    • Snippets
  • Members
    • Members
  • Collapse sidebar
  • Activity
  • Graph
  • Charts
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
  • GNOME
  • gjsgjs
  • Merge Requests
  • !50

Merged
Opened Jan 31, 2018 by Philip Chimento@ptomato 
  • Report abuse
Report abuse

object: don't use toggle references unless necessary

Many GObjects (such as widgets and actors) don't have JS state, but they're kept alive by C code. We can therefore save some memory by GCing these objects, and creating new wrappers when needed. If state is ever set, we transparently switch to toggle refs, so no change should be visible at the JS level.

Closes #62 (closed).

Edited Apr 12, 2018 by Georges Basile Stavracas Neto
  • Discussion 8
  • Commits 2
  • Pipelines 5
  • Changes 1
{{ resolvedDiscussionCount }}/{{ discussionCount }} {{ resolvedCountText }} resolved
  • Philip Chimento @ptomato

    mentioned in issue #140 (closed)

    Mar 31, 2018

    mentioned in issue #140 (closed)

    mentioned in issue #140
    Toggle commit list
  • Philip Chimento @ptomato

    mentioned in merge request !114 (merged)

    Apr 07, 2018

    mentioned in merge request !114 (merged)

    mentioned in merge request !114
    Toggle commit list
  • Georges Basile Stavracas Neto @feaneron

    added 188 commits

    • 3dec6f78...e9f4c12e - 186 commits from branch master
    • a89b7014 - object: don't use toggle references unless necessary
    • 6116ee0b - object: properly disassociate wrappers

    Compare with previous version

    Apr 07, 2018

    added 188 commits

    • 3dec6f78...e9f4c12e - 186 commits from branch master
    • a89b7014 - object: don't use toggle references unless necessary
    • 6116ee0b - object: properly disassociate wrappers

    Compare with previous version

    added 188 commits <ul><li>3dec6f78...e9f4c12e - 186 commits from branch <code>master</code></li><li>a89b7014 - object: don&#39;t use toggle references unless necessary</li><li>6116ee0b - object: properly disassociate wrappers</li></ul> [Compare with previous version](https://gitlab.gnome.org/GNOME/gjs/merge_requests/50/diffs?diff_id=5374&start_sha=3dec6f78642369756df4950a465669f1fd8d4ef4)
    Toggle commit list
  • Georges Basile Stavracas Neto @feaneron

    added 4 commits

    • 6116ee0b...6357e4cd - 2 commits from branch master
    • fe372193 - object: don't use toggle references unless necessary
    • eee20edf - object: properly disassociate wrappers

    Compare with previous version

    Apr 07, 2018

    added 4 commits

    • 6116ee0b...6357e4cd - 2 commits from branch master
    • fe372193 - object: don't use toggle references unless necessary
    • eee20edf - object: properly disassociate wrappers

    Compare with previous version

    added 4 commits <ul><li>6116ee0b...6357e4cd - 2 commits from branch <code>master</code></li><li>fe372193 - object: don&#39;t use toggle references unless necessary</li><li>eee20edf - object: properly disassociate wrappers</li></ul> [Compare with previous version](https://gitlab.gnome.org/GNOME/gjs/merge_requests/50/diffs?diff_id=5375&start_sha=6116ee0b312de5078592050ab05a44bed894395b)
    Toggle commit list
  • Philip Chimento
    @ptomato started a discussion on an outdated change in commit eee20edf Apr 09, 2018
    Automatically resolved by Georges Basile Stavracas Neto with a push Apr 12, 2018
    gi/object.cpp
    1583 1588 GJS_DEC_COUNTER(object);
    1584 1589 priv->~ObjectInstance();
    1585 1590 g_slice_free(ObjectInstance, priv);
    1591
    1592 /* Remove the ObjectInstace pointer from the JSObject */
    1593 JS_SetPrivate(obj, nullptr);
    1594 g_assert(JS_GetPrivate(obj) == nullptr);
    • Philip Chimento @ptomato commented Apr 09, 2018
      Master

      This assertion is probably redundant. I hope we can assume the JS_SetPrivate() call directly above works as documented!

      This assertion is probably redundant. I hope we can assume the `JS_SetPrivate()` call directly above works as documented!
    • Georges Basile Stavracas Neto @feaneron

      changed this line in version 4 of the diff

      Apr 12, 2018

      changed this line in version 4 of the diff

      changed this line in [version 4 of the diff](https://gitlab.gnome.org/GNOME/gjs/merge_requests/50/diffs?diff_id=5625&start_sha=eee20edf3bc82e776f6737abe0b50838f042462d#fd9d2d7a82fce609fc319a328f60bf4ee4bc26df_1594_1598)
      Toggle commit list
    Please register or sign in to reply
  • Philip Chimento
    @ptomato started a discussion on an old version of the diff Apr 09, 2018
    Automatically resolved by Georges Basile Stavracas Neto with a push Apr 12, 2018
    gi/object.cpp
    70 70
    71 71 unsigned js_object_finalized : 1;
    72 72 unsigned g_object_finalized : 1;
    73
    74 /* True if this object has visible JS state, and thus its lifecycle is
    75 * managed using toggle references. False if this object just keeps a
    76 * hard ref on the underlying GObject, and may be finalized at will. */
    77 bool uses_toggle_ref;
    • Philip Chimento @ptomato commented Apr 09, 2018
      Master

      I'd probably make this bool uses_toggle_ref : 1; to pack more closely with the above.

      I'd probably make this `bool uses_toggle_ref : 1;` to pack more closely with the above.
    • Georges Basile Stavracas Neto @feaneron

      changed this line in version 4 of the diff

      Apr 12, 2018

      changed this line in version 4 of the diff

      changed this line in [version 4 of the diff](https://gitlab.gnome.org/GNOME/gjs/merge_requests/50/diffs?diff_id=5625&start_sha=eee20edf3bc82e776f6737abe0b50838f042462d#fd9d2d7a82fce609fc319a328f60bf4ee4bc26df_77_77)
      Toggle commit list
    Please register or sign in to reply
  • Philip Chimento
    @ptomato started a discussion on an old version of the diff Apr 09, 2018
    Automatically resolved by Georges Basile Stavracas Neto with a push Apr 12, 2018
    gi/object.cpp
    521 523 return result.succeed();
    522 524 }
    523 525
    526 if (!gjs_get_string_id(context, id, &name)) {
    527 /* We need to keep the wrapper alive in order not to loose custom
    • Philip Chimento @ptomato commented Apr 09, 2018
      Master

      loose -> lose

      loose -> lose
    • Georges Basile Stavracas Neto @feaneron

      changed this line in version 4 of the diff

      Apr 12, 2018

      changed this line in version 4 of the diff

      changed this line in [version 4 of the diff](https://gitlab.gnome.org/GNOME/gjs/merge_requests/50/diffs?diff_id=5625&start_sha=eee20edf3bc82e776f6737abe0b50838f042462d#fd9d2d7a82fce609fc319a328f60bf4ee4bc26df_527_527)
      Toggle commit list
    Please register or sign in to reply
  • Philip Chimento @ptomato

    mentioned in issue #62 (closed)

    Apr 10, 2018

    mentioned in issue #62 (closed)

    mentioned in issue #62
    Toggle commit list
  • Philip Chimento @ptomato

    changed milestone to GNOME 3.30.0

    Apr 10, 2018

    changed milestone to GNOME 3.30.0

    changed milestone to %4
    Toggle commit list
  • Georges Basile Stavracas Neto @feaneron

    resolved all discussions

    Apr 12, 2018

    resolved all discussions

    resolved all discussions
    Toggle commit list
  • Georges Basile Stavracas Neto @feaneron

    added 23 commits

    • eee20edf...4a28c6b2 - 21 commits from branch master
    • 6f67afaf - object: don't use toggle references unless necessary
    • 88b5c7ef - object: properly disassociate wrappers

    Compare with previous version

    Apr 12, 2018

    added 23 commits

    • eee20edf...4a28c6b2 - 21 commits from branch master
    • 6f67afaf - object: don't use toggle references unless necessary
    • 88b5c7ef - object: properly disassociate wrappers

    Compare with previous version

    added 23 commits <ul><li>eee20edf...4a28c6b2 - 21 commits from branch <code>master</code></li><li>6f67afaf - object: don&#39;t use toggle references unless necessary</li><li>88b5c7ef - object: properly disassociate wrappers</li></ul> [Compare with previous version](https://gitlab.gnome.org/GNOME/gjs/merge_requests/50/diffs?diff_id=5625&start_sha=eee20edf3bc82e776f6737abe0b50838f042462d)
    Toggle commit list
  • Georges Basile Stavracas Neto @feaneron commented Apr 12, 2018
    Developer

    The pipeline failure seems completely unrelated (it's failing to find a Docker image)

    The pipeline failure seems completely unrelated (it's failing to find a Docker image)
  • Georges Basile Stavracas Neto @feaneron

    unmarked as a Work In Progress

    Apr 12, 2018

    unmarked as a Work In Progress

    unmarked as a **Work In Progress**
    Toggle commit list
  • Philip Chimento
    @ptomato started a discussion on an old version of the diff Apr 16, 2018
    Automatically resolved by Georges Basile Stavracas Neto with a push Apr 17, 2018
    gi/object.cpp
    1556 1592 GJS_DEC_COUNTER(object);
    1557 1593 priv->~ObjectInstance();
    1558 1594 g_slice_free(ObjectInstance, priv);
    1595
    1596 /* Remove the ObjectInstace pointer from the JSObject */
    • Philip Chimento @ptomato commented Apr 16, 2018
      Master

      typo: ObjectInstance

      typo: ObjectInstance
    • Georges Basile Stavracas Neto @feaneron

      changed this line in version 5 of the diff

      Apr 17, 2018

      changed this line in version 5 of the diff

      changed this line in [version 5 of the diff](https://gitlab.gnome.org/GNOME/gjs/merge_requests/50/diffs?diff_id=5857&start_sha=88b5c7ef468c31518313b8184c070b69cd00f69b#fd9d2d7a82fce609fc319a328f60bf4ee4bc26df_1596_1596)
      Toggle commit list
    Please register or sign in to reply
  • Philip Chimento
    @ptomato started a discussion on an old version of the diff Apr 16, 2018
    Automatically resolved by Georges Basile Stavracas Neto with a push Apr 17, 2018
    gi/object.cpp
    1215 1232
    1216 1233 for (auto iter = wrapped_gobject_list.begin(); iter != wrapped_gobject_list.end(); ) {
    1217 1234 ObjectInstance *priv = *iter;
    1218 if (priv->keep_alive.rooted() || priv->keep_alive == nullptr ||
    1235 if (priv->keep_alive == nullptr || priv->keep_alive.rooted() ||
    • Philip Chimento @ptomato commented Apr 16, 2018
      Master

      Did the static analyzer suggest this? If so, we should rewrite it to not trip that heuristic, for the reasons given in !121 (diffs, comment 97355)

      Did the static analyzer suggest this? If so, we should rewrite it to not trip that heuristic, for the reasons given in https://gitlab.gnome.org/GNOME/gjs/merge_requests/121/diffs#note_97355
    • Philip Chimento @ptomato commented Apr 16, 2018
      Master

      Although, this seems to work fine, unlike inverting the conditions...

      Although, this seems to work fine, unlike inverting the conditions...
    • Georges Basile Stavracas Neto @feaneron

      changed this line in version 5 of the diff

      Apr 17, 2018

      changed this line in version 5 of the diff

      changed this line in [version 5 of the diff](https://gitlab.gnome.org/GNOME/gjs/merge_requests/50/diffs?diff_id=5857&start_sha=88b5c7ef468c31518313b8184c070b69cd00f69b#fd9d2d7a82fce609fc319a328f60bf4ee4bc26df_1235_1235)
      Toggle commit list
    Please register or sign in to reply
  • Philip Chimento @ptomato commented Apr 16, 2018
    Master

    I tested this with JS_GC_ZEAL, and it works fine. Should be ready to merge after these minor comments.

    I tested this with `JS_GC_ZEAL`, and it works fine. Should be ready to merge after these minor comments.
  • Georges Basile Stavracas Neto @feaneron

    resolved all discussions

    Apr 17, 2018

    resolved all discussions

    resolved all discussions
    Toggle commit list
  • Georges Basile Stavracas Neto @feaneron

    added 16 commits

    • 88b5c7ef...ead64625 - 14 commits from branch master
    • 0cc23474 - object: don't use toggle references unless necessary
    • 72d970b4 - object: properly disassociate wrappers

    Compare with previous version

    Apr 17, 2018

    added 16 commits

    • 88b5c7ef...ead64625 - 14 commits from branch master
    • 0cc23474 - object: don't use toggle references unless necessary
    • 72d970b4 - object: properly disassociate wrappers

    Compare with previous version

    added 16 commits <ul><li>88b5c7ef...ead64625 - 14 commits from branch <code>master</code></li><li>0cc23474 - object: don&#39;t use toggle references unless necessary</li><li>72d970b4 - object: properly disassociate wrappers</li></ul> [Compare with previous version](https://gitlab.gnome.org/GNOME/gjs/merge_requests/50/diffs?diff_id=5857&start_sha=88b5c7ef468c31518313b8184c070b69cd00f69b)
    Toggle commit list
  • Philip Chimento @ptomato

    mentioned in commit a6b6fc13

    Apr 18, 2018

    mentioned in commit a6b6fc13

    mentioned in commit a6b6fc1342b76e6c536483a655f3c54dd8d27653
    Toggle commit list
  • Philip Chimento @ptomato

    merged

    Apr 18, 2018

    merged

    merged
    Toggle commit list
  • Georges Basile Stavracas Neto
    @feaneron started a discussion on commit 0cc23474 Apr 20, 2018
    Last updated by Georges Basile Stavracas Neto Apr 20, 2018
    • Georges Basile Stavracas Neto @feaneron

      mentioned in commit 8d50e2b4

      Apr 20, 2018

      mentioned in commit 8d50e2b4

      mentioned in commit 8d50e2b401236a380bb16301308b331413217a2d
      Toggle commit list
    • Georges Basile Stavracas Neto @feaneron

      mentioned in merge request !127 (merged)

      Apr 20, 2018

      mentioned in merge request !127 (merged)

      mentioned in merge request !127
      Toggle commit list
    Please register or sign in to reply
  • Write
  • Preview
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment
Assignee
No assignee
Assign to
GNOME 3.30.0
Milestone
GNOME 3.30.0
Assign milestone
Time tracking
0
Labels
None
Assign labels
  • View labels
Reference: GNOME/gjs!50
×

Revert this merge request

This will create a new commit in order to revert the existing changes.

Switch branch
Cancel
A new branch will be created in your fork and a new merge request will be started.
×

Cherry-pick this merge request

Switch branch
Cancel
A new branch will be created in your fork and a new merge request will be started.