[jira] [Created] (YUNIKORN-1721) Re-think the usage of *resource.Resource objects

["Peter Bacsko (Jira)" <ji...@apache.org>]

Peter Bacsko created YUNIKORN-1721:
--------------------------------------

             Summary: Re-think the usage of *resource.Resource objects
                 Key: YUNIKORN-1721
                 URL: https://issues.apache.org/jira/browse/YUNIKORN-1721
             Project: Apache YuniKorn
          Issue Type: Sub-task
          Components: core - scheduler
            Reporter: Peter Bacsko


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 {{*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.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@yunikorn.apache.org
For additional commands, e-mail: dev-help@yunikorn.apache.org