Re: [OE-core] [PATCH V2] sqlite3: Revert ad601c7962 from 3.14.1 amalgamation package

2016-11-10 Thread Patrick Ohly 
On Thu, 2016-11-10 at 18:53 +0100, Patrick Ohly wrote:
> On Thu, 2016-10-13 at 13:16 -0700, Jianxun Zhang wrote:
> > It turns out this change between 3.12.2 and 3.13 introduces
> > a 2% increase of build time based on statistic data in
> > bz10367.
> 
> Let me add that this patch increased build performance in Ostro even
> more: apparently one big impact of the sqlite performance issue is on
> pseudo. Ostro depends fairly heavily on pseudo because of meta-swupd and
> xattrs on all files.
> 
> When this patch and others recently landed in Ostro, total build times
> dropped from 4:46h (build #508,
> https://ostroproject.org/jenkins/job/build_intel-corei7-64/2763/console)
> to 2:07h (build #510,
> https://ostroproject.org/jenkins/job/build_intel-corei7-64/2831/console).

In case that someone wonders: in addition to that enhancement, we can
still do even better by also reducing the work that meta-swupd causes. A
build with my recent meta-swupd enhancements takes the build time down
to 1:05h.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.



-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [OE-core] [PATCH V2] sqlite3: Revert ad601c7962 from 3.14.1 amalgamation package

2016-11-10 Thread Jussi Kukkonen 
On 10 November 2016 at 19:53, Patrick Ohly  wrote:

> On Thu, 2016-10-13 at 13:16 -0700, Jianxun Zhang wrote:
> > It turns out this change between 3.12.2 and 3.13 introduces
> > a 2% increase of build time based on statistic data in
> > bz10367.
>
> Let me add that this patch increased build performance in Ostro even
> more: apparently one big impact of the sqlite performance issue is on
> pseudo. Ostro depends fairly heavily on pseudo because of meta-swupd and
> xattrs on all files.
>
> When this patch and others recently landed in Ostro, total build times
> dropped from 4:46h (build #508,
> https://ostroproject.org/jenkins/job/build_intel-corei7-64/2763/console)
> to 2:07h (build #510,
> https://ostroproject.org/jenkins/job/build_intel-corei7-64/2831/console).
>
> That could also be because of other improvements, perhaps even in our CI
> hardware, so take those numbers with a large chunk of salt.
>
> However, in local builds with and without this patch (i.e. everything
> else the same) I also see big differences for pseudo-heavy operations:
>
> $ buildstats-diff --diff-attr walltime --min-val 60 with-patch/
> without-patch/
> Ignoring tasks less than 01:00.0 (60.0s)
> Ignoring differences less than 00:02.0 (2.0s)
>
>   PKG TASK
> ABSDIFF   RELDIFF  WALLTIME1 -> WALLTIME2
> ...
>   bundle-ostro-image-swupd-qa-bundle-bdo_rootfs
>  78.1s   +115.4%  67.7s -> 145.8s
>   bundle-ostro-image-swupd-qa-bundle-ado_rootfs
>  80.3s   +116.8%  68.8s -> 149.1s
>   bundle-ostro-image-swupd-qa-bundle-ado_image
>  106.8s   +291.9%  36.6s -> 143.3s
>   bundle-ostro-image-swupd-qa-bundle-bdo_image
>  107.9s   +298.2%  36.2s -> 144.1s
>   bundle-ostro-image-swupd-mega   do_image
>  244.4s+74.2% 329.2s -> 573.6s
>   bundle-ostro-image-swupd-world-dev  do_rootfs
> 246.7s   +207.2% 119.1s -> 365.8s
>   bundle-ostro-image-swupd-world-dev  do_image
>  269.2s+83.5% 322.6s -> 591.7s
>   bundle-ostro-image-swupd-mega   do_rootfs
> 272.6s   +246.1% 110.8s -> 383.3s
>   ostro-image-swupd   do_rootfs
> 676.1s   +808.1%  83.7s -> 759.8s
>   bundle-ostro-image-swupd-world-dev  do_copy_bundle_contents
>  1339.5s  +2957.6%  45.3s -> 1384.8s
>   bundle-ostro-image-swupd-qa-bundle-bdo_copy_bundle_contents
>  1475.0s  +3147.8%  46.9s -> 1521.9s
>   bundle-ostro-image-swupd-qa-bundle-ado_copy_bundle_contents
>  1503.9s  +3283.0%  45.8s -> 1549.8s
>
> Cumulative walltime:
>   6070.9s+326.9%30:57.3 (1857.3s) -> 2:12:08.2 (7928.2s)
>

yikes.

So it really seems that the sqlite change is a very relevant
> improvement.
>
> That leads me to a bigger question: has upstream been notified about
> this?
>
> Our observation may also be relevant to other sqlite users. Besides, not
> getting this fixed upstream means that we'll have to do the same tricky
> revert for the next upstream version update.
>

Completely true. It may also be worthwhile to profile what pseudo is really
doing. The feeling I had was that there could be something going wrong
there as recipes that install lots of files not only take very long but
also create _very_ large database file: The increases do not seem even
close to linear as one might expect. Seebs gave some advice about this in
the bug (https://bugzilla.yoctoproject.org/show_bug.cgi?id=10367#c11).

Jussi




> --
> Best Regards, Patrick Ohly
>
> The content of this message is my personal opinion only and although
> I am an employee of Intel, the statements I make here in no way
> represent Intel's position on the issue, nor am I authorized to speak
> on behalf of Intel on this matter.
>
>
>
> --
> ___
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core
>
-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

Re: [OE-core] [PATCH V2] sqlite3: Revert ad601c7962 from 3.14.1 amalgamation package

2016-11-10 Thread Patrick Ohly 
On Thu, 2016-10-13 at 13:16 -0700, Jianxun Zhang wrote:
> It turns out this change between 3.12.2 and 3.13 introduces
> a 2% increase of build time based on statistic data in
> bz10367.

Let me add that this patch increased build performance in Ostro even
more: apparently one big impact of the sqlite performance issue is on
pseudo. Ostro depends fairly heavily on pseudo because of meta-swupd and
xattrs on all files.

When this patch and others recently landed in Ostro, total build times
dropped from 4:46h (build #508,
https://ostroproject.org/jenkins/job/build_intel-corei7-64/2763/console)
to 2:07h (build #510,
https://ostroproject.org/jenkins/job/build_intel-corei7-64/2831/console).

That could also be because of other improvements, perhaps even in our CI
hardware, so take those numbers with a large chunk of salt.

However, in local builds with and without this patch (i.e. everything
else the same) I also see big differences for pseudo-heavy operations:

$ buildstats-diff --diff-attr walltime --min-val 60 with-patch/ without-patch/
Ignoring tasks less than 01:00.0 (60.0s)
Ignoring differences less than 00:02.0 (2.0s)

  PKG TASK  ABSDIFF   
RELDIFF  WALLTIME1 -> WALLTIME2
...
  bundle-ostro-image-swupd-qa-bundle-bdo_rootfs   78.1s   
+115.4%  67.7s -> 145.8s   
  bundle-ostro-image-swupd-qa-bundle-ado_rootfs   80.3s   
+116.8%  68.8s -> 149.1s   
  bundle-ostro-image-swupd-qa-bundle-ado_image   106.8s   
+291.9%  36.6s -> 143.3s   
  bundle-ostro-image-swupd-qa-bundle-bdo_image   107.9s   
+298.2%  36.2s -> 144.1s   
  bundle-ostro-image-swupd-mega   do_image   244.4s
+74.2% 329.2s -> 573.6s   
  bundle-ostro-image-swupd-world-dev  do_rootfs  246.7s   
+207.2% 119.1s -> 365.8s   
  bundle-ostro-image-swupd-world-dev  do_image   269.2s
+83.5% 322.6s -> 591.7s   
  bundle-ostro-image-swupd-mega   do_rootfs  272.6s   
+246.1% 110.8s -> 383.3s   
  ostro-image-swupd   do_rootfs  676.1s   
+808.1%  83.7s -> 759.8s   
  bundle-ostro-image-swupd-world-dev  do_copy_bundle_contents   1339.5s  
+2957.6%  45.3s -> 1384.8s  
  bundle-ostro-image-swupd-qa-bundle-bdo_copy_bundle_contents   1475.0s  
+3147.8%  46.9s -> 1521.9s  
  bundle-ostro-image-swupd-qa-bundle-ado_copy_bundle_contents   1503.9s  
+3283.0%  45.8s -> 1549.8s  

Cumulative walltime:
  6070.9s+326.9%30:57.3 (1857.3s) -> 2:12:08.2 (7928.2s)

So it really seems that the sqlite change is a very relevant
improvement.

That leads me to a bigger question: has upstream been notified about
this?

Our observation may also be relevant to other sqlite users. Besides, not
getting this fixed upstream means that we'll have to do the same tricky
revert for the next upstream version update.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.



-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core

[OE-core] [PATCH V2] sqlite3: Revert ad601c7962 from 3.14.1 amalgamation package

2016-10-13 Thread Jianxun Zhang 
It turns out this change between 3.12.2 and 3.13 introduces
a 2% increase of build time based on statistic data in
bz10367.

The added patch is forged by diffing the new sqlite3.c
generated from reverting the change in raw source of sqlite3
project, and then manually migrate the delta to a sqlite3.c
from the 3.14.1 tarball package because what recipes reference
is actually a generated C code (amalgamation) release package
and we cannot apply the real change to 3.14.1 cleanly due to
so many changes happened.

Fixes [YOCTO #10367]

Signed-off-by: Jianxun Zhang 
---
V2 adds "Upstream Status: Inappropriate" in the commit msg in the patch file.

Thanks to Saul's reminder.

 ...1c7962-that-brings-2-increase-of-build-ti.patch | 56 ++
 meta/recipes-support/sqlite/sqlite3_3.14.1.bb  |  5 +-
 2 files changed, 60 insertions(+), 1 deletion(-)
 create mode 100644 
meta/recipes-support/sqlite/files/0001-revert-ad601c7962-that-brings-2-increase-of-build-ti.patch

diff --git 
a/meta/recipes-support/sqlite/files/0001-revert-ad601c7962-that-brings-2-increase-of-build-ti.patch
 
b/meta/recipes-support/sqlite/files/0001-revert-ad601c7962-that-brings-2-increase-of-build-ti.patch
new file mode 100644
index 000..26540b2
--- /dev/null
+++ 
b/meta/recipes-support/sqlite/files/0001-revert-ad601c7962-that-brings-2-increase-of-build-ti.patch
@@ -0,0 +1,56 @@
+From 4b286b441e8efa9a34eb0db8227748ebffd91c35 Mon Sep 17 00:00:00 2001
+From: Jianxun Zhang 
+Date: Thu, 13 Oct 2016 09:24:21 -0700
+Subject: [PATCH] revert ad601c7962 that brings 2% increase of build time.
+
+The comment of the change in sqlite fossil project is:
+"For in-memory databases, it does not matter if pcache
+entries are marked "clean" or "writable"."
+
+Upstream Status: Inappropriate
+
+Signed-off-by: Jianxun Zhang 
+---
+ sqlite3.c | 12 +++-
+ 1 file changed, 3 insertions(+), 9 deletions(-)
+
+diff --git a/sqlite3.c b/sqlite3.c
+index ccddfe6..ecae550 100644
+--- a/sqlite3.c
 b/sqlite3.c
+@@ -13146,7 +13146,7 @@ struct PgHdr {
+   sqlite3_pcache_page *pPage;/* Pcache object page handle */
+   void *pData;   /* Page data */
+   void *pExtra;  /* Extra content */
+-  PgHdr *pDirty; /* Transient list of dirty sorted by pgno */
++  PgHdr *pDirty; /* Transient list of dirty pages */
+   Pager *pPager; /* The pager this page is part of */
+   Pgno pgno; /* Page number for this page */
+ #ifdef SQLITE_CHECK_PAGES
+@@ -43504,13 +43504,7 @@ bitvec_end:
+ /* #include "sqliteInt.h" */
+ 
+ /*
+-** A complete page cache is an instance of this structure.  Every
+-** entry in the cache holds a single page of the database file.  The
+-** btree layer only operates on the cached copy of the database pages.
+-**
+-** A page cache entry is "clean" if it exactly matches what is currently
+-** on disk.  A page is "dirty" if it has been modified and needs to be
+-** persisted to disk.
++** A complete page cache is an instance of this structure.
+ **
+ ** pDirty, pDirtyTail, pSynced:
+ **   All dirty pages are linked into the doubly linked list using
+@@ -48314,7 +48308,7 @@ static int pager_end_transaction(Pager *pPager, int 
hasMaster, int bCommit){
+   pPager->pInJournal = 0;
+   pPager->nRec = 0;
+   if( rc==SQLITE_OK ){
+-if( pagerFlushOnCommit(pPager, bCommit) ){
++if( MEMDB || pagerFlushOnCommit(pPager, bCommit) ){
+   sqlite3PcacheCleanAll(pPager->pPCache);
+ }else{
+   sqlite3PcacheClearWritable(pPager->pPCache);
+-- 
+2.7.4
+
diff --git a/meta/recipes-support/sqlite/sqlite3_3.14.1.bb 
b/meta/recipes-support/sqlite/sqlite3_3.14.1.bb
index 3af0d2f..7c8fa40 100644
--- a/meta/recipes-support/sqlite/sqlite3_3.14.1.bb
+++ b/meta/recipes-support/sqlite/sqlite3_3.14.1.bb
@@ -3,7 +3,10 @@ require sqlite3.inc
 LICENSE = "PD"
 LIC_FILES_CHKSUM = 
"file://sqlite3.h;endline=11;md5=65f0a57ca6928710b418c094b3570bb0"
 
-SRC_URI = "http://www.sqlite.org/2016/sqlite-autoconf-${SQLITE_PV}.tar.gz;
+SRC_URI = "\
+  http://www.sqlite.org/2016/sqlite-autoconf-${SQLITE_PV}.tar.gz \
+  file://0001-revert-ad601c7962-that-brings-2-increase-of-build-ti.patch \
+  "
 
 SRC_URI[md5sum] = "3634a90a3f49541462bcaed3474b2684"
 SRC_URI[sha256sum] = 
"bc7182476900017becb81565ecea7775d46ab747a97281aa610f4f45881c47a6"
-- 
2.7.4

-- 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core