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 DAB227511B6; Tue, 9 Jan 2024 16:54:34 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org DAB227511B6 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1704808475; bh=hWo8LHF0D8g0rdJ99rhZs6fZDqXaqXD9rbczczYALu4=; 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=gxYhlSyxOS5rHI84YmORBwP8YnnAvD4BVex6z9RqykcjSGPw38MICCwazYA/XAgkt K7LV988CpNAtBaLnBOF1IssmQl/nrIJavrfidH8kr+EVxz0w7yeYHAlkLqor+3UE5b VCRMbHa/TiGbYTzSStYBSAO8cX2MEAWGVbr61/CY= Received: from smtp52.i.mail.ru (smtp52.i.mail.ru [95.163.41.88]) (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 B12547511B6 for ; Tue, 9 Jan 2024 16:54:33 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org B12547511B6 Received: by smtp52.i.mail.ru with esmtpa (envelope-from ) id 1rNCYq-006zcL-1G; Tue, 09 Jan 2024 16:54:32 +0300 Date: Tue, 9 Jan 2024 16:54:32 +0300 To: Sergey Bronnikov Message-ID: References: <4fb82034fcac21359f79b81cc6643054fc432be3.1701888856.git.m.kokryashkin@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Mailru-Src: smtp X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9F008C97756F746CA9853402441EE20032119B858DB59C778182A05F538085040915EC49174728AF008A0D443C2C31A59353305E6A39796EBF3E9AFA13A624D44 X-C1DE0DAB: 0D63561A33F958A5EF5A37516323F3AEEA0CBAA9B01DC5080C883B46308A393FF87CCE6106E1FC07E67D4AC08A07B9B067F1C1C3ABB44F3ACB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF77DD89D51EBB7742D3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF390B5883004FAB4B20D908FB09334164C482CAF41D84DAF938E352B85ABC499780BEFAEC9D2720085550A38C5D2E6452F5DBEA753CBF9938CADED01F198A78E2A74DFFEFA5DC0E7F02C26D483E81D6BE64ACE4A408B72B61B0CA6F94E606A667A52EF62A646584F811BD90D3D42C882D43082AE146A756F3 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojou3Ii6fsdvW35NB1nr+vHA== X-Mailru-Sender: 11C2EC085EDE56FA38FD4C59F7EFE407A61FD8AAC939F9AFBA3C6F57D37ADD49ED760379E96079F6D51284F0FE6F529ABC7555A253F5B200DF104D74F62EE79D27EC13EC74F6107F4198E0F3ECE9B5443453F38A29522196 X-Mras: OK Subject: Re: [Tarantool-patches] [PATCH luajit v2 6/6] profilers: print user-friendly errors 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: Maxim Kokryashkin via Tarantool-patches Reply-To: Maxim Kokryashkin Cc: Maxim Kokryashkin , tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Sergey! Thanks for the review! Fixed your minor, branch is force-pushed. Here is the diff: === diff --git a/tools/memprof/parse.lua b/tools/memprof/parse.lua index cc66a23e..11a79a1d 100644 --- a/tools/memprof/parse.lua +++ b/tools/memprof/parse.lua @@ -206,12 +206,13 @@ function M.parse(reader, symbols) local _ = reader:read_octets(3) if magic ~= LJM_MAGIC then - error("Bad LJM format prologue: "..magic) + error("Bad memprof event format prologue: "..magic) end if string.byte(version) ~= LJM_CURRENT_VERSION then error(string_format( - "LJM format version mismatch: the tool expects %d, but your data is %d", + "Memprof event format version mismatch:".. + " the tool expects %d, but your data is %d", LJM_CURRENT_VERSION, string.byte(version) )) diff --git a/tools/sysprof/parse.lua b/tools/sysprof/parse.lua index 749f70db..0e416d37 100755 --- a/tools/sysprof/parse.lua +++ b/tools/sysprof/parse.lua @@ -237,12 +237,13 @@ function M.parse(reader, symbols) local _ = reader:read_octets(3) if magic ~= LJP_MAGIC then - error("Bad LJP format prologue: " .. tostring(magic)) + error("Bad sysprof event format prologue: " .. tostring(magic)) end if string.byte(version) ~= LJP_CURRENT_VERSION then error(string_format( - "LJP format version mismatch: the tool expects %d, but your data is %d", + "Sysprof event format version mismatch:".. + " the tool expects %d, but your data is %d", LJP_CURRENT_VERSION, string.byte(version) )) diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua index c4aefef7..e80b9f33 100644 --- a/tools/utils/symtab.lua +++ b/tools/utils/symtab.lua @@ -95,13 +95,13 @@ function M.parse(reader) local _ = reader:read_octets(3) if magic ~= LJS_MAGIC then - error("Bad LJS format prologue: " .. tostring(magic)) + error("Bad LuaJIT symbol table format prologue: " .. tostring(magic)) end if string.byte(version) ~= LJS_CURRENT_VERSION then error(string_format( - "LJS format version mismatch:".. - "the tool expects %d, but your data is %d", + "LuaJIT symbol table format version mismatch:".. + " the tool expects %d, but your data is %d", LJS_CURRENT_VERSION, string.byte(version) )) === WBR, Max On Fri, Dec 29, 2023 at 03:27:46PM +0300, Sergey Bronnikov wrote: > Hello, Max > > > thanks for the patch. LGTM with minor comments > > > On 12/6/23 21:55, Maxim Kokryashkin wrote: > > > > > > diff --git a/tools/sysprof/parse.lua b/tools/sysprof/parse.lua > > index 64c4a455..749f70db 100755 > > --- a/tools/sysprof/parse.lua > > +++ b/tools/sysprof/parse.lua > > @@ -237,7 +237,7 @@ function M.parse(reader, symbols) > > local _ = reader:read_octets(3) > > if magic ~= LJP_MAGIC then > > - error("Bad LJP format prologue: "..magic) > > + error("Bad LJP format prologue: " .. tostring(magic)) > > end > > LJP is not a well-known abbreviation. Probably, it is better to write the > error > > message without it. I see that LJP/LJS is used extensively in error message, > so feel free to > > ignore or fix in a separate patch. > > > if string.byte(version) ~= LJP_CURRENT_VERSION then > > diff --git a/tools/utils/safe_event_reader.lua b/tools/utils/safe_event_reader.lua > > new file mode 100644 > > index 00000000..39246a9d > > --- /dev/null > > +++ b/tools/utils/safe_event_reader.lua > > @@ -0,0 +1,34 @@ > > +local bufread = require('utils.bufread') > > +local symtab = require('utils.symtab') > > + > > +local function make_error_handler(fmt, inputfile) > > + return function(err) > > + io.stderr:write(string.format(fmt, inputfile)) > > + io.stderr:write(string.format('\nOriginal error: %s', err)) > > + os.exit(1, true) > > + end > > +end > > + > > +local function safe_event_reader(parser, inputfile) > > + local _, reader = xpcall( > > + bufread.new, > > + make_error_handler('Failed to open %s.', inputfile), > > + inputfile > > + ) > > + > > + local _, symbols = xpcall( > > + symtab.parse, > > + make_error_handler('Failed to parse symtab from %s.', inputfile), > > + reader > > + ) > > + > > + local _, events = xpcall( > > + parser, > > + make_error_handler('Failed to parse profile data from %s.', inputfile), > > + reader, > > + symbols > > + ) > > + return events, symbols > > +end > > + > > +return safe_event_reader > > diff --git a/tools/utils/symtab.lua b/tools/utils/symtab.lua > > index 0c878e7a..c4aefef7 100644 > > --- a/tools/utils/symtab.lua > > +++ b/tools/utils/symtab.lua > > @@ -95,7 +95,7 @@ function M.parse(reader) > > local _ = reader:read_octets(3) > > if magic ~= LJS_MAGIC then > > - error("Bad LJS format prologue: "..magic) > > + error("Bad LJS format prologue: " .. tostring(magic)) > > LJS is not a well-known abbreviation. Probably, it is better to write the > error > > message without it. > > > end > > if string.byte(version) ~= LJS_CURRENT_VERSION then