From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 57762D6F05F; Mon, 9 Sep 2024 16:37:20 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 57762D6F05F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1725889040; bh=doGrc3DqZII19Gw9hnqybyPkk7p5v6t6CJYy+4CBpU0=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=UbdnjAoF5d1et89ZrRk3lW4kKR62VSQs+sdanmPIkoCDgHRW9/tQeXtOJK3QwSfoe tT7Nwq6zfi1yJgrADC8Ma08QPzgQApFf09/2N6oedpDZL3eQdMoMnpaGFM2H8UxfS0 2ADF4/k8+s5rLPA/DwgdXgXuYhJO00LJthKPxtQc= Received: from smtp38.i.mail.ru (smtp38.i.mail.ru [95.163.41.79]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id AD4F94A6671 for ; Mon, 9 Sep 2024 16:37:18 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org AD4F94A6671 Received: by smtp38.i.mail.ru with esmtpa (envelope-from ) id 1sneZx-0000000GmIm-1Ftq; Mon, 09 Sep 2024 16:37:17 +0300 Content-Type: multipart/alternative; boundary="------------SFlHg56ArKysqRrJWVDfCfQm" Message-ID: <165129ca-9bd3-4cb9-b106-5d6e26fe4847@tarantool.org> Date: Mon, 9 Sep 2024 16:37:17 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Sergey Kaplun References: <20240902125421.16727-1-skaplun@tarantool.org> In-Reply-To: X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9FD43264444B6060C9433EF2A07CEE9B0C8E037688117B9DA182A05F538085040E0EBC33857594E35479CDAE959BF6424AF865EF0160B91A1D6802897B205AACF79EDB787894F0DC7 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7A0175C48BD57B26BC2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE7CE015DA2C687B387EA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38B043BF0FB74779F36206E8287DE38736CB7797925CBEA9C7832CD165E848F2864A471835C12D1D9774AD6D5ED66289B5259CC434672EE6371117882F4460429724CE54428C33FAD30A8DF7F3B2552694AC26CFBAC0749D213D2E47CDBA5A9658378DA827A17800CE7328B01A8D746D8839FA2833FD35BB23DF004C90652538430302FCEF25BFAB3454AD6D5ED66289B5278DA827A17800CE772E36DBF6487CDDCD32BA5DBAC0009BE395957E7521B51C2330BD67F2E7D9AF1090A508E0FED6299176DF2183F8FC7C040FEA8D7F2827269CD04E86FAF290E2DB606B96278B59C421DD303D21008E29813377AFFFEAFD269176DF2183F8FC7C0FE3A47D6FA29121068655334FD4449CB33AC447995A7AD186D8C47C27EEC5E9FD5E8D9A59859A8B65D673F6B91EFC66D089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A5F5113AC2A56F35135002B1117B3ED69627E2B94F4A9294BE47A99E6294EE8661823CB91A9FED034534781492E4B8EEAD85CCBA673D36D1A4BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742424CF958EAFF5D571004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D3472E5ECC12A9739C17B6F2442B8209BF10D14FAEA353BC8585B59184F0188E53788B38E64698D4B9A1D7E09C32AA3244C85EFDAD7225E80025DAF20B2795D9467035AF4853A73314BEA455F16B58544A2557BDE0DD54B3590A5AE236DF995FB59978A700BF655EAEEED6A17656DB59BCAD427812AF56FC65B X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojvtJrZAoTnsOmWFqI9ldJyQ== X-Mailru-Sender: 520A125C2F17F0B1E52FEF5D219D6140CF5C30A90A1F15E292C4E9A5CFD972EE5812D3B20B23105B0152A3D17938EB451EB5A0BCEC6A560B3DDE9B364B0DF289BE2DA36745F2EEB5CEBA01FB949A1F1EEAB4BC95F72C04283CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] FFI: Drop finalizer table rehash after GC cycle. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" This is a multi-part message in MIME format. --------------SFlHg56ArKysqRrJWVDfCfQm Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Hi, Sergey, thanks for the fixes. Please replace "grop" to "grow" in commit message. LGTM On 07.09.2024 08:15, Sergey Kaplun wrote: > Hi, Sergey! > Thanks for the review! > Fixed your comments, rebased the branch on the tarantool/master and > force-pushed it. > > On 06.09.24, Sergey Bronnikov wrote: >> Hi, Sergey! >> >> >> thanks for the patch! LGTM with minor comments below >> >> On 02.09.2024 15:54, Sergey Kaplun wrote: >>> From: Mike Pall >>> >>> Reported by Sergey Kaplun. >>> >>> (cherry picked from commit fb22d0f80f291827a4004e16bc589b54bcc4a3c7) >>> >>> The raising of the OOM error when rehashing the finalizer table (when we >>> can't allocate a new hash part) leads to crashes in either >>> `lj_trace_exit()` or `lj_trace_unwind()` due to unprotected error >>> raising, which either has no DWARF eh_frame or loses the context of the >> I would add a link to a page about eh_frame, for example [1] >> >> Feel free to ignore. >> >> 1. >> https://refspecs.linuxfoundation.org/LSB_3.0.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html > Added, thanks. > >>> JIT compiler. >>> >>> This patch drops rehashing of the finalizer table to avoid these >> I would replace "finalizer" with "cdata finalizer". >> >> And I would say about drawbacks of this. Otherwise, it looks like > Added. > See the new commit message below: > > | FFI: Drop finalizer table rehash after GC cycle. > | > | Reported by Sergey Kaplun. > | > | (cherry picked from commit fb22d0f80f291827a4004e16bc589b54bcc4a3c7) > | > | The raising of the OOM error when rehashing the finalizer table (when we > | can't allocate a new hash part) leads to crashes in either > | `lj_trace_exit()` or `lj_trace_unwind()` due to unprotected error > | raising, which either has no DWARF eh_frame [1] or loses the context of > | the JIT compiler. > | > | This patch drops rehashing of the cdata finalizer table to avoid these > | crashes. It will prevent the cdata finalizer table from shrinking when > | the huge amount of the cdata objects is collected by the GC. OTOH, the > | finzlizer table most probably will grop anyway to the old size, so this > | is not crucial. > | > | Sergey Kaplun: > | * added the description and the test for the problem > | > | [1]:https://refspecs.linuxfoundation.org/LSB_3.0.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html > | > | Part of tarantool/tarantool#10199 > | Resolves tarantool/tarantool#10290 > >> rehashing was not needed from the beginning. >> >>> crashes. >>> >>> Sergey Kaplun: >>> * added the description and the test for the problem >>> >>> Part of tarantool/tarantool#10199 >>> Resolves tarantool/tarantool#10290 >> Usually "Closes" or "Fixes". Feel free to ignore. > For LuaJIT, we always use "Resolves", since it is __closed__ when the > submodule is bumped in the release branches and master. > >>> --- >>> >>> Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1247-fin-tab-rehashing-on-trace >>> Related Issues: >>> *https://github.com/tarantool/tarantool/issues/10290 >>> *https://github.com/LuaJIT/LuaJIT/issues/1247 >>> *https://github.com/tarantool/tarantool/issues/10199 >>> > > --------------SFlHg56ArKysqRrJWVDfCfQm Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: 7bit

Hi, Sergey,


thanks for the fixes. Please replace "grop" to "grow" in commit

message. LGTM

On 07.09.2024 08:15, Sergey Kaplun wrote:
Hi, Sergey!
Thanks for the review!
Fixed your comments, rebased the branch on the tarantool/master and
force-pushed it.

On 06.09.24, Sergey Bronnikov wrote:

Hi, Sergey!


thanks for the patch! LGTM with minor comments below

On 02.09.2024 15:54, Sergey Kaplun wrote:

From: Mike Pall <mike>

Reported by Sergey Kaplun.

(cherry picked from commit fb22d0f80f291827a4004e16bc589b54bcc4a3c7)

The raising of the OOM error when rehashing the finalizer table (when we
can't allocate a new hash part) leads to crashes in either
`lj_trace_exit()` or `lj_trace_unwind()` due to unprotected error
raising, which either has no DWARF eh_frame or loses the context of the

I would add a link to a page about eh_frame, for example [1]

Feel free to ignore.

1. 
https://refspecs.linuxfoundation.org/LSB_3.0.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html

Added, thanks.



        

          
JIT compiler.

This patch drops rehashing of the finalizer table to avoid these


        

        
I would replace "finalizer" with "cdata finalizer".

And I would say about drawbacks of this. Otherwise, it looks like


      

      
Added.
See the new commit message below:

| FFI: Drop finalizer table rehash after GC cycle.
|
| Reported by Sergey Kaplun.
|
| (cherry picked from commit fb22d0f80f291827a4004e16bc589b54bcc4a3c7)
|
| The raising of the OOM error when rehashing the finalizer table (when we
| can't allocate a new hash part) leads to crashes in either
| `lj_trace_exit()` or `lj_trace_unwind()` due to unprotected error
| raising, which either has no DWARF eh_frame [1] or loses the context of
| the JIT compiler.
|
| This patch drops rehashing of the cdata finalizer table to avoid these
| crashes. It will prevent the cdata finalizer table from shrinking when
| the huge amount of the cdata objects is collected by the GC. OTOH, the
| finzlizer table most probably will grop anyway to the old size, so this
| is not crucial.
|
| Sergey Kaplun:
| * added the description and the test for the problem
|
| [1]: https://refspecs.linuxfoundation.org/LSB_3.0.0/LSB-Core-generic/LSB-Core-generic/ehframechpt.html
|
| Part of tarantool/tarantool#10199
| Resolves tarantool/tarantool#10290



      

        
rehashing was not needed from the beginning.



        

          
crashes.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#10199
Resolves tarantool/tarantool#10290


        

        
Usually "Closes" or "Fixes". Feel free to ignore.


      

      
For LuaJIT, we always use "Resolves", since it is __closed__ when the
submodule is bumped in the release branches and master.



      

        

          
---

Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1247-fin-tab-rehashing-on-trace
Related Issues:
*https://github.com/tarantool/tarantool/issues/10290
*https://github.com/LuaJIT/LuaJIT/issues/1247
*https://github.com/tarantool/tarantool/issues/10199



        

      

      
<snipped>



    

  
  


--------------SFlHg56ArKysqRrJWVDfCfQm--