From kriszyp@gmail.com Fri Feb 21 03:39:19 2020 From: kriszyp@gmail.com To: openldap-bugs@openldap.org Subject: RE: RE: Re: (ITS#9017) Improving performance of commit sync in Windows Date: Fri, 21 Feb 2020 03:39:17 +0000 Message-ID: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============4598034224129841076==" --===============4598034224129841076== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable

Here is a pull request for these w= in=3D dows upgrades (I know it won=3DE2=3D80=3D99t be applied directly in GitHub, b= ut s=3D eems a convenient host for the patches):

https:= =3D //github.com/LMDB/lmdb/pull/18

 

The first commit is basically t= =3D he changes mentioned in this thread for improving sync performance by using=3D write-through writes instead FlushFileBuffers/FlushViewOfFile, reapplied t=3D o the mdb.master branch, with conflicts resolved.

<= =3D a href=3D3D"https://github.com/LMDB/lmdb/commit/204d1710d7a34eff142f2654d7678= =3D 182e1b9e054.patch">https://github.com/LMDB/lm= =3D db/commit/204d1710d7a34eff142f2654d7678182e1b9e054.patch

 

The second com= mi=3D t adds support for an option for opening map files by providing the mapsize=3D as the file size (MaximumSize) to NtCreateSection (which also improves per=3D formance and prevents freezing caused by file growth). I implemented this a=3D s a compiler preprocessor define, as requested. However, I would note that =3D I would still prefer this to be a runtime option. When bundling LMDB in a n=3D ode package, there is no way to parameterize the compiler options (that I k=3D now of) for distribution, and it would be nice to have this an option for d=3D ifferent users of the node package.

https://github.com/LMDB/lmdb/commit/6f94= =3D bb1da1812c1ddb68e33448c9cd381674b02d.patch

 

However, in trying to test the= =3D mdb.master branch code with our application/server (we were previously usi=3D ng the 0.9 branch), there was a regression causing it to crash pretty much =3D any time we attempted to write to a db that was larger than 2GB. After some=3D considerable testing and investigation (which is why it took a while to ge=3D t this finished), it seems the cause is the use of the off_t type for file =3D pointer/positions, which appears to be a 32-bit signed integer compiling on=3D Windows. This overflows for dbs over 2GB and causes references that crash =3D the process. Replacing all the off_t types with size_t (unsigned 64-bit), f=3D ixed this issue. I am not sure if this is the right replacement type. I thi=3D nk this could also be addressed with compiler option for changing the file =3D offset type size, but that seems like a hazardous hoop to jump through. But=3D the third commit, replacing the off_t with size_t definitely fixed the pro=3D blem in our application.

https://github.com/LMDB/lmdb/commit/45cf4b6ede38565= =3D cfab1c40d0d77961a0cb22b9e.patch

&nb= =3D sp;

With these three commits we are seeing so= =3D lid stability and performance on Windows with the master branch. Thanks for=3D considering this!

 

Thanks,
Kris

 

From: kriszyp(a)= gmai=3D l.com
Sent: February 18, 2020 1:06 PM
To: Howard Chu
Cc: openldap-its(a)OpenLDAP.org
Subject: R=3D E: Re: (ITS#9017) Improving performance of commit sync in Windows

=3D

 

Looking in= to=3D the APIs, I would guess that CreateFileMapping=3DE2=3D80=3D99s dwMaximumSize= High=3D and dwMaximumSizeLow arguments are mapped to NtCreateSection=3DE2=3D80=3D99s= Max=3D imumSize argument (4th argument). In LMDB=3DE2=3D80=3D99s 0.9 Crea= teFi=3D leMapping call, the size is provided (since it is required), whereas NtCrea=3D teSection has MaximumSize as optional, and in LMDB (mdb.master), it is not =3D provided. Passing the size in (with MaximumSize argument) to NtCreateSectio=3D n seems to result in the same behavior as CreateFileMapping of preallocatin=3D g a file size equivalent to the max/mapping size, rather than dynamically g=3D rowing.

 

It is challenging to readily test for the performance issues; it =3D took a fair bit of effort and load testing to trigger the long-duration per=3D formance/freezing issues we had seen before on Azure. However, in some more=3D isolated tests, providing a MaximumSize to NtCreateSection does seem to pe=3D rform better than leaving it unspecified (specifically the system CPU usage=3D seems to about be about twice as much with dynamic growth than fixed file =3D size), and the performance seems about the same as CreateFileMapping. And I=3D think we both had suspected that it was likely due to the overhead or inef=3D ficiencies in how Windows was dynamically growing the (size of the) mapped =3D file, and having Windows used a predefined fixed file size, whether through=3D NtCreateSection or CreateFileMapping, probably would prevent these slowdow=3D n issues.

 

But, whether we use NtCreateSection or CreateFileMapping, we a= =3D re still faced with the same trade-off of whether to use dynamic Windows-co=3D ntrolled file growth (convenient since the map size can be set arbitrarily =3D large), or using pre-specified file size (to avoid the slowing/freezing tha=3D t seems to sometimes result from dynamic Windows growth). So I assume it wo=3D uld still be reasonable to offer a compile time option, so users could opt-=3D in to using a fixed file size (provide the MaximumSize to NtCreateSection)?=3D Fortunately this would be a trivial addition, just a few lines of code to =3D use the preprocessor switch to supply this argument.

 

http://undocument=3D ed.ntinternals.net/index.html?page=3D3DUserMode%2FUndocumented%20Functions%2F= =3D NT%20Objects%2FSection%2FNtMapViewOfSection.html

 

Thanks,
Kris=3D

 

From: Howard Chu
S=3D ent:
February 18, 2020 8:15 AM
To: Kris Zyp
Cc: openldap-its(a)OpenLDAP.org
Subject: Re: (ITS#9017) Imp= =3D roving performance of commit sync in Windows

 

Kris Zyp wrote:=3D

> Yes, I will work on upgrading this patc= =3D h to 1.0.

>

> However, I believe in order to realize optimal Windows pe= =3D rformance without regression/slow-downs, I will also need to disable the us=3D e of NTDLL (and use direct

> CreateFi= =3D leMapping memory maps instead), as mentioned  http://www.openldap.org/=3D lists/openldap-bugs/201902/msg00009.html. You had mentioned in that thread =3D that

> the NTDLL code wasn't destined= =3D for release, is that still the case (it is still in the mdb.master branch)=3D ? If you are intending to release (1.0) with the

> NTDLL-based memory maps (which is understandable, can certain=3D ly be an easier path for Windows users getting started), I will also need t=3D o add the compiler

> option to disabl= =3D e it. Anyway, I will work on creating a PR with both these patches.

 

Mig= ht=3D be interesting to investigate why the behavior with NTDLL is so much slowe=3D r on Azure. Since

the Win32 APIs are imp= =3D lemented on top of the NT APIs, you would expect them to perform identicall=3D y.

Perhaps there's some additional flag = =3D that the Win32 API is passing to NT that is needed on Azure.

=3D

 

> =

> Thanks,

> Kris

>

> On Mon, Feb 17, 2020 at 8:14 AM Howard Chu <hyc(a)syma= s.=3D com <mailto:hyc(a)symas.com>> wrote:

>

>     K= =3D ris Zyp wrote:

>   &nb= =3D sp; > Sorry to keep pestering, but just pinging about this patch again, =3D as I still think this fix could benefit windows users. And at this point, I=3D think I can

>    = =3D ; say we

>     &g= =3D t; have tested it pretty well, running on our servers for almost a year :).=3D

>

>     Looks like this patch is against the 0.9 rel=3D ease branch. I hit a bunch of conflicts

= =3D >     trying to apply it to mdb.master. We'll be sto=3D pping work on 0.9 soon, and getting

>= =3D      LMDB 1.0 out the door finally, so can you please v=3D erify that your changes will work

>&n= =3D bsp;    on mdb.master as well?

>

>    = =3D > Thanks,

>   &nbs= =3D p; > Kris

>    = =3D ; >

>     >= =3D On Wed, Sep 18, 2019 at 12:56 PM Kris Zyp <kriszyp(a)gmail.com <mailto= =3D :kriszyp(a)gmail.com> <mailto:kriszyp(a)gmail.com <mailto:kriszyp(a)= gmai=3D l.com>>> wrote:

>  = =3D ;   >

>   = =3D ;  >     Checking on this again, is this still a pos=3D sibility for merging into LMDB? This fix is still working great (improved p=3D erformance) on our systems.

> &n= =3D bsp;   >     Thanks,

>     >     Kris

>     >

= =3D

>     >     O= =3D n Mon, Jun 17, 2019 at 1:04 PM Kris Zyp <kriszyp(a)gmail.com <mailto:kr= =3D iszyp(a)gmail.com> <mailto:kriszyp(a)gmail.com <mailto:kriszyp(a)gma= il.c=3D om>>> wrote:

>  &n= =3D bsp;  >

>   &n= =3D bsp; >         Is this still being considered/r=3D eviewed? Let me know if there are any other changes you would like me to ma=3D ke. This patch has continued to yield

&g= =3D t;     >         significan=3D t and reliable performance improvements for us, and seems like it would be =3D nice for this to be available for other Windows users.

>     >

>     >         =3D ;On Fri, May 3, 2019 at 3:52 PM Kris Zyp <kriszyp(a)gmail.com <mailto:k= =3D riszyp(a)gmail.com> <mailto:kriszyp(a)gmail.com <mailto:kriszyp(a)gm= ail.=3D com>>> wrote:

>  &= =3D nbsp;  >

>   &= =3D nbsp; >             For the sake of p=3D utting this in the email thread (other code discussion in GitHub), here is =3D the latest squashed commit of the proposed patch (with

>     >       = =3D      the on-demand, retained overlapped array to reduce re-=3D malloc and opening event handles):

>&= =3D nbsp;    >             =3D ;https://github.com/kriszyp/node-lmdb/commit/726a9156662c703bf3d453aab75ee2=3D 22072b990f

>     = =3D >

>

>

>    = =3D --

>      = =3D -- Howard Chu

>   &nb= =3D sp;   CTO, Symas Corp.           http://=3D www.symas.com

>   &nbs= =3D p;   Director, Highland Sun     http://highlandsun.com/=3D hyc/

>      = =3D Chief Architect, OpenLDAP  http://www.openldap.org/project/

>

&= nb=3D sp;

 

--

  -- Howard Chu

  CTO, Symas Corp.    &n= =3D bsp;      http://www.symas.com

  Director, Highland Sun     http:= =3D //highlandsun.com/hyc/

  Chief Arch= =3D itect, OpenLDAP  http://www.openldap.org/project/

 

 <=3D /p>=3D --===============4598034224129841076==--