Uploaded image for project: 'Mesos'
  1. Mesos
  2. MESOS-1991

Remove dynamic allocation from Option

Attach filesAttach ScreenshotVotersWatch issueWatchersLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Improvement
    • Status: Resolved
    • Minor
    • Resolution: Fixed
    • None
    • 0.23.0
    • stout

    Description

      Remove dynamic allocations from Option class.

      The motivation for this is 3-fold:
      1. Reduce dynamic allocations. These can cause latency jitter as process lifetime grows. This kind of jitter can make it hard to grasp the upper bound of latency on certain operations under locks. This modification only moves the allocated space of T, it does not reduce or increase the number of actual construction / move calls unless the new move constructor is used.
      2. The commonly understood implication of Optional / Option / Nullable is that it augments the type field by 1 bit in order to allow representation of an unknown or null state. This is handy in cases where a type such as int64_t fully utilizes its 64 bit storage space, and representing unknown would otherwise require us to steal a number (such as INT64_MAX). This class should not take on the additional responsibility of managing memory for the augmented type.
      3. It can be very deceptive to a newcomer when Option<int64_t> does a dynamic allocation. Intuitively you would not expect a type such as int64_t to do a dynamic allocation or be expensive to copy. Naturally Option<BigType> would be expected to be expensive to copy, and so a developer would be more inclined to do something like std::shared_ptr<Option<BigType>>.

      Attachments

        Issue Links

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            jvanremoortere Joris Van Remoortere
            jvanremoortere Joris Van Remoortere
            Benjamin Mahler Benjamin Mahler
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment