[PATCH 1/5] sstate.bbclass: track found files on mirrors with a counter

[PATCH 1/5] sstate.bbclass: track found files on mirrors with a counter

[Jose Quaresma]

We don't need extra python collections to count the found files
on the sstate cache and sstate mirrors.
The main found collections provides all the files that were found,
then we only need to count the files on sstate mirror

Signed-off-by: Jose Quaresma <quaresma.jose@gmail.com>
---
 meta/classes/sstate.bbclass | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
index 2175ace4c4..2575750247 100644
--- a/meta/classes/sstate.bbclass
+++ b/meta/classes/sstate.bbclass
@@ -871,8 +871,6 @@ BB_HASHCHECK_FUNCTION = "sstate_checkhashes"
 
 def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0, summary=True, **kwargs):
     found = set()
-    foundLocal = set()
-    foundNet = set()
     missed = set()
 
     def gethash(task):
@@ -905,12 +903,11 @@ def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0, summary=True,
         if os.path.exists(sstatefile):
             bb.debug(2, "SState: Found valid sstate file %s" % sstatefile)
             found.add(tid)
-            foundLocal.add(tid)
-            continue
         else:
-            missed.add(tid)
             bb.debug(2, "SState: Looked for but didn't find file %s" % sstatefile)
+            missed.add(tid)
 
+    foundMirrors = 0
     mirrors = d.getVar("SSTATE_MIRRORS")
     if mirrors:
         # Copy the data object and override DL_DIR and SRC_URI
@@ -950,8 +947,9 @@ def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0, summary=True,
                             connection_cache=thread_worker.connection_cache)
                 fetcher.checkstatus()
                 bb.debug(2, "SState: Successful fetch test for %s" % srcuri)
+                foundMirrors += 1
                 found.add(tid)
-                foundNet.add(tid)
+
                 if tid in missed:
                     missed.remove(tid)
             except:
@@ -1013,7 +1011,8 @@ def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0, summary=True,
         match = 0
         if total:
             match = len(found) / total * 100
-        bb.plain("Sstate summary: Wanted %d Local %d Network %d Missed %d Current %d (%d%% match, %d%% complete)" % (total, len(foundLocal), len(foundNet),len(missed), currentcount, match, complete))
+        bb.plain("Sstate summary: Wanted %d Local %d Mirrors %d Missed %d Current %d (%d%% match, %d%% complete)" %
+            (total, len(found)-foundMirrors, foundMirrors, len(missed), currentcount, match, complete))
 
     if hasattr(bb.parse.siggen, "checkhashes"):
         bb.parse.siggen.checkhashes(sq_data, missed, found, d)
-- 
2.32.0

[PATCH 2/5] sstate.bbclass: only search on the mirrors for the missing files

[Jose Quaresma]

On the first search we found some files on the local sstate cache.
The missing files are know as well when this step finish.
When we have sstate mirrors we don't need to iterate all files again
because we already know what's missing.

Signed-off-by: Jose Quaresma <quaresma.jose@gmail.com>
---
 meta/classes/sstate.bbclass | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
index 2575750247..c3c145e7f3 100644
--- a/meta/classes/sstate.bbclass
+++ b/meta/classes/sstate.bbclass
@@ -949,11 +949,8 @@ def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0, summary=True,
                 bb.debug(2, "SState: Successful fetch test for %s" % srcuri)
                 foundMirrors += 1
                 found.add(tid)
-
-                if tid in missed:
-                    missed.remove(tid)
+                missed.remove(tid)
             except:
-                missed.add(tid)
                 bb.debug(2, "SState: Unsuccessful fetch test for %s" % srcuri)
                 pass
             if len(tasklist) >= min_tasks:
@@ -961,9 +958,7 @@ def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0, summary=True,
 
         tasklist = []
         min_tasks = 100
-        for tid in sq_data['hash']:
-            if tid in found:
-                continue
+        for tid in missed:
             spec, extrapath, tname = getpathcomponents(tid, d)
             sstatefile = d.expand(extrapath + generate_sstatefn(spec, gethash(tid), tname, siginfo, d))
             tasklist.append((tid, sstatefile))
-- 
2.32.0

[PATCH 3/5] sstate.bbclass: get the number of threads with cpu_count from oe utils

[Jose Quaresma]

It uses the python os.sched_getaffinity and it is more acurrate

Signed-off-by: Jose Quaresma <quaresma.jose@gmail.com>
---
 meta/classes/sstate.bbclass | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
index c3c145e7f3..63085a7f3a 100644
--- a/meta/classes/sstate.bbclass
+++ b/meta/classes/sstate.bbclass
@@ -964,13 +964,12 @@ def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0, summary=True,
             tasklist.append((tid, sstatefile))
 
         if tasklist:
+            nproc = min(oe.utils.cpu_count(), len(tasklist))
+
             if len(tasklist) >= min_tasks:
                 msg = "Checking sstate mirror object availability"
                 bb.event.fire(bb.event.ProcessStarted(msg, len(tasklist)), d)
 
-            import multiprocessing
-            nproc = min(multiprocessing.cpu_count(), len(tasklist))
-
             bb.event.enable_threadlock()
             pool = oe.utils.ThreadedPool(nproc, len(tasklist),
                     worker_init=checkstatus_init, worker_end=checkstatus_end)
-- 
2.32.0

[PATCH 4/5] sstate.bbclass: disable thread lock if we don't have events

[Jose Quaresma]

commit f2053844958325496a9387874a8f3182400b71ca
'classes/sstate.bbclass: Enable thread lock when checkstatus'
adds a thread lock to don't lose the events from multiple threads
that runs on the ThreadPool.

commit 1444b8a2ae226829e719d3d184fca27e5940ae0d
'sstate.bbclass: Only show sstate mirror progress bar for >= 100 objects'
disable the events if we don't have a minium number of objects.

So we can only use the thread lock when we have the events in place.

Signed-off-by: Jose Quaresma <quaresma.jose@gmail.com>
---
 meta/classes/sstate.bbclass | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
index 63085a7f3a..948779386d 100644
--- a/meta/classes/sstate.bbclass
+++ b/meta/classes/sstate.bbclass
@@ -969,17 +969,17 @@ def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0, summary=True,
             if len(tasklist) >= min_tasks:
                 msg = "Checking sstate mirror object availability"
                 bb.event.fire(bb.event.ProcessStarted(msg, len(tasklist)), d)
+                bb.event.enable_threadlock()
 
-            bb.event.enable_threadlock()
             pool = oe.utils.ThreadedPool(nproc, len(tasklist),
                     worker_init=checkstatus_init, worker_end=checkstatus_end)
             for t in tasklist:
                 pool.add_task(checkstatus, t)
             pool.start()
             pool.wait_completion()
-            bb.event.disable_threadlock()
 
             if len(tasklist) >= min_tasks:
+                bb.event.disable_threadlock()
                 bb.event.fire(bb.event.ProcessFinished(msg), d)
 
     inheritlist = d.getVar("INHERIT")
-- 
2.32.0

Re: [OE-core] [PATCH 4/5] sstate.bbclass: disable thread lock if we don't have events

[Richard Purdie]

On Mon, 2021-08-09 at 10:48 +0100, Jose Quaresma wrote:
> commit f2053844958325496a9387874a8f3182400b71ca
> 'classes/sstate.bbclass: Enable thread lock when checkstatus'
> adds a thread lock to don't lose the events from multiple threads
> that runs on the ThreadPool.
> 
> commit 1444b8a2ae226829e719d3d184fca27e5940ae0d
> 'sstate.bbclass: Only show sstate mirror progress bar for >= 100 objects'
> disable the events if we don't have a minium number of objects.
> 
> So we can only use the thread lock when we have the events in place.
> 
> Signed-off-by: Jose Quaresma <quaresma.jose@gmail.com>
> ---
>  meta/classes/sstate.bbclass | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
> index 63085a7f3a..948779386d 100644
> --- a/meta/classes/sstate.bbclass
> +++ b/meta/classes/sstate.bbclass
> @@ -969,17 +969,17 @@ def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0, summary=True,
>              if len(tasklist) >= min_tasks:
>                  msg = "Checking sstate mirror object availability"
>                  bb.event.fire(bb.event.ProcessStarted(msg, len(tasklist)), d)
> +                bb.event.enable_threadlock()
>  
> 
> 
> 
> -            bb.event.enable_threadlock()
>              pool = oe.utils.ThreadedPool(nproc, len(tasklist),
>                      worker_init=checkstatus_init, worker_end=checkstatus_end)
>              for t in tasklist:
>                  pool.add_task(checkstatus, t)
>              pool.start()
>              pool.wait_completion()
> -            bb.event.disable_threadlock()
>  
> 
> 
> 
>              if len(tasklist) >= min_tasks:
> +                bb.event.disable_threadlock()
>                  bb.event.fire(bb.event.ProcessFinished(msg), d)
>  
> 

This patch series somehow breaks the build in weird ways, typically:

https://autobuilder.yoctoproject.org/typhoon/#/builders/59/builds/3789

i.e. the sdk-update from an eSDK somehow fails to get sstate objects from the 
network. Michael and I spent hours trying to figure out what was breaking
things and whether this was an infrastructure issue with sstate or not :(.

I dropped this series and builds look much happier. I think this patch
isn't correct as the threadlock isn't guarding against just progress
events, it guards against any events from threads. There are debug messages
in the code which turn into events too. As such, when threads are in use, 
the lock is needed.

I therefore suspect it is this change that breaks things in weird ways
but I haven't confirmed that.

Cheers,

Richard

Re: [OE-core] [PATCH 4/5] sstate.bbclass: disable thread lock if we don't have events

[Jose Quaresma]

First of all, sorry for the inconvenience of this patch that adds many troubles.

I have concluded from the commit history that the lock is introduced
because of the progress bar and I don't know that the debug message
uses the events when it runs inside threads.

Basically I am wrong in my assumption and this patch doesn't make sense,
so I will remove it from the series and send the others that are minor tweaks
and little improvements in the sstate mirror.

Best regards,
José Quaresma

Richard Purdie <richard.purdie@linuxfoundation.org> escreveu no dia
quarta, 11/08/2021 à(s) 20:40:
>
> On Mon, 2021-08-09 at 10:48 +0100, Jose Quaresma wrote:
> > commit f2053844958325496a9387874a8f3182400b71ca
> > 'classes/sstate.bbclass: Enable thread lock when checkstatus'
> > adds a thread lock to don't lose the events from multiple threads
> > that runs on the ThreadPool.
> >
> > commit 1444b8a2ae226829e719d3d184fca27e5940ae0d
> > 'sstate.bbclass: Only show sstate mirror progress bar for >= 100 objects'
> > disable the events if we don't have a minium number of objects.
> >
> > So we can only use the thread lock when we have the events in place.
> >
> > Signed-off-by: Jose Quaresma <quaresma.jose@gmail.com>
> > ---
> >  meta/classes/sstate.bbclass | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
> > index 63085a7f3a..948779386d 100644
> > --- a/meta/classes/sstate.bbclass
> > +++ b/meta/classes/sstate.bbclass
> > @@ -969,17 +969,17 @@ def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0, summary=True,
> >              if len(tasklist) >= min_tasks:
> >                  msg = "Checking sstate mirror object availability"
> >                  bb.event.fire(bb.event.ProcessStarted(msg, len(tasklist)), d)
> > +                bb.event.enable_threadlock()
> >
> >
> >
> >
> > -            bb.event.enable_threadlock()
> >              pool = oe.utils.ThreadedPool(nproc, len(tasklist),
> >                      worker_init=checkstatus_init, worker_end=checkstatus_end)
> >              for t in tasklist:
> >                  pool.add_task(checkstatus, t)
> >              pool.start()
> >              pool.wait_completion()
> > -            bb.event.disable_threadlock()
> >
> >
> >
> >
> >              if len(tasklist) >= min_tasks:
> > +                bb.event.disable_threadlock()
> >                  bb.event.fire(bb.event.ProcessFinished(msg), d)
> >
> >
>
> This patch series somehow breaks the build in weird ways, typically:
>
> https://autobuilder.yoctoproject.org/typhoon/#/builders/59/builds/3789
>
> i.e. the sdk-update from an eSDK somehow fails to get sstate objects from the
> network. Michael and I spent hours trying to figure out what was breaking
> things and whether this was an infrastructure issue with sstate or not :(.
>
> I dropped this series and builds look much happier. I think this patch
> isn't correct as the threadlock isn't guarding against just progress
> events, it guards against any events from threads. There are debug messages
> in the code which turn into events too. As such, when threads are in use,
> the lock is needed.
>
> I therefore suspect it is this change that breaks things in weird ways
> but I haven't confirmed that.
>
> Cheers,
>
> Richard
>


-- 
Best regards,

José Quaresma

[PATCH 5/5] sstate.bbclass: sstate mirror progress bar cleanup

[Jose Quaresma]

We only has the progress bar when we have more than 100 objects.
So check for this and store the result to show the progress bar.

Signed-off-by: Jose Quaresma <quaresma.jose@gmail.com>
---
 meta/classes/sstate.bbclass | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/meta/classes/sstate.bbclass b/meta/classes/sstate.bbclass
index 948779386d..a9c908fe97 100644
--- a/meta/classes/sstate.bbclass
+++ b/meta/classes/sstate.bbclass
@@ -953,20 +953,24 @@ def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0, summary=True,
             except:
                 bb.debug(2, "SState: Unsuccessful fetch test for %s" % srcuri)
                 pass
-            if len(tasklist) >= min_tasks:
+
+            if progress:
                 bb.event.fire(bb.event.ProcessProgress(msg, len(tasklist) - thread_worker.tasks.qsize()), d)
 
         tasklist = []
-        min_tasks = 100
         for tid in missed:
             spec, extrapath, tname = getpathcomponents(tid, d)
             sstatefile = d.expand(extrapath + generate_sstatefn(spec, gethash(tid), tname, siginfo, d))
             tasklist.append((tid, sstatefile))
 
+        progress = False
+        if len(tasklist) >= 100:
+            progress = True
+
         if tasklist:
             nproc = min(oe.utils.cpu_count(), len(tasklist))
 
-            if len(tasklist) >= min_tasks:
+            if progress:
                 msg = "Checking sstate mirror object availability"
                 bb.event.fire(bb.event.ProcessStarted(msg, len(tasklist)), d)
                 bb.event.enable_threadlock()
@@ -978,7 +982,7 @@ def sstate_checkhashes(sq_data, d, siginfo=False, currentcount=0, summary=True,
             pool.start()
             pool.wait_completion()
 
-            if len(tasklist) >= min_tasks:
+            if progress:
                 bb.event.disable_threadlock()
                 bb.event.fire(bb.event.ProcessFinished(msg), d)
 
-- 
2.32.0