XMLWordPrintableJSON

Details

    • Sub-task
    • Status: Closed
    • Major
    • Resolution: Won't Do
    • None
    • None
    • core - scheduler
    • None

    Description

      We had a discussion with wilfreds about how we use the Resource data type in the core. There are some things that we can do to improve performance.

      It would be ideal to use it as a pure value, but K8s have a bunch of resource types already, plus we want to support any kind of resource, so using a map is inevitable. We could handle more common resources like cpu, memory, pods as separate fields to avoid map creation in many cases, but this complicates the code. We need to indicate if a resource is set or not, so we either need sentinel values or boolean fields like haveCpu, haveMemory, etc (this can be solved by generating the source based on a template, but we probably don't want to go this route).

      But there are still things that we can do.

      #1 resource.Resource is basically a Go map, so passing it around as pointer makes little sense. It's an extra indirection which isn't needed. This makes the object graph in the heap more complicated and affects the GC. We should handle this as a value type, although cloning is still necessary. Mutation is possible via pointer receiver methods like (*Resource).AddTo(). The only restriction is that mutation syntax is not available in map values:

      myMap[somekey].AddTo(r)

      In this case, we need to write some extra code (assign to variable, mutate, re-assign), which is an acceptable tradeoff. Looking at the current code, we don't have existing code like that.

      #2 We should prefer mutation if possible. Since Resource does not compose a mutex, it should not be shared among goroutines (that's why we have Clone(). But there are places where we update values like:

      sa.pending = resources.Add(sa.pending, delta)

      We can just rewrite this to sa.pending.AddTo(delta), unless there's a good reason not to do so.

      This is a considerable amount of work, especialy #1, so we can split this up further.

      Attachments

        Activity

          People

            Unassigned Unassigned
            pbacsko Peter Bacsko
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: