Skip to content

Commit 82a8b39

Browse files
serge1serge1
committed
Optimization - use reference_wrapper<> for returned ario::Member parameters. It supposed to reduce the number of data copy
1 parent a4c005d commit 82a8b39

File tree

3 files changed

+96
-47
lines changed

3 files changed

+96
-47
lines changed

‎ario/ario.hpp‎

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,6 @@ class ario
7272
friend class ario;
7373

7474
public:
75-
explicit Member() {}
76-
7775
std::string name = {}; ///< Name of the member
7876
int date = {}; ///< Date of the member
7977
int uid = {}; ///< User ID of the member
@@ -322,16 +320,21 @@ class ario
322320
//! @brief Find a symbol in the archive
323321
//! @param name The name of the symbol to find
324322
//! @param out_member Pointer to store the found member
323+
//! @param member Reference to store the found member
325324
//! @return Error object indicating success or failure
326325
//! If the symbol is found, out_member will point to the corresponding member
327326
//! If the symbol is not found, out_member will stay unchanged
328-
Result find_symbol( std::string_view name, ario::Member& member ) const
327+
Result
328+
find_symbol( std::string_view name,
329+
std::optional<std::reference_wrapper<const ario::Member>>&
330+
member ) const
329331
{
330332
const auto it = symbol_table.find( std::string( name ) );
331333
if ( it != symbol_table.end() ) {
332334
member = members_[it->second];
333335
return {};
334336
}
337+
member = std::nullopt;
335338
return { std::string( "Symbol not found: " ) + std::string( name ) };
336339
}
337340

@@ -367,28 +370,42 @@ class ario
367370
}
368371

369372
//------------------------------------------------------------------------------
370-
//! @brief
371-
Result add_member( const Member& m, const std::string& data )
373+
//! @brief Add a member to the archive
374+
//! @param member The member to add
375+
//! @param data The data associated with the member
376+
//! @param added_member Reference to store the added member
377+
//! @return Error object indicating success or failure
378+
Result
379+
add_member( const Member& member,
380+
const std::string& data,
381+
std::optional<std::reference_wrapper<const ario::Member>>&
382+
added_member )
372383
{
373-
auto& new_member = members_.emplace_back( m );
384+
auto& new_member = members_.emplace_back( member );
374385
new_member.size = data.size();
375386
new_member.pstream = nullptr;
376387
new_member.set_new_data( data );
377388

378-
if ( m.name.size() < FIELD_NAME_SIZE ) {
379-
new_member.short_name = m.name + "/";
389+
if ( member.name.size() < FIELD_NAME_SIZE ) {
390+
new_member.short_name = member.name + "/";
380391
}
381392
else {
382393
auto location = string_table.size();
383-
string_table += m.name + "/\x0A";
394+
string_table += member.name + "/\x0A";
384395
new_member.short_name = "/" + std::to_string( location );
385396
}
386397

398+
added_member = new_member;
387399
return {};
388400
}
389401

390-
Result add_symbols_for_member(const ario::Member& member,
391-
const std::vector<std::string>& symbols)
402+
//------------------------------------------------------------------------------
403+
//! @brief Add symbols for a member
404+
//! @param member The member to add symbols for
405+
//! @param symbols The symbols to add
406+
//! @return Error object indicating success or failure
407+
Result add_symbols_for_member( const ario::Member& member,
408+
const std::vector<std::string>& symbols )
392409
{
393410
std::string_view member_name = member.name;
394411
size_t index = std::distance(
@@ -401,15 +418,13 @@ class ario
401418
return { "Member not found in archive" };
402419
}
403420

404-
for (const auto& symbol : symbols)
405-
{
421+
for ( const auto& symbol : symbols ) {
406422
symbol_table[symbol] = index;
407423
}
408424

409425
return {};
410426
}
411427

412-
413428
protected:
414429
//------------------------------------------------------------------------------
415430
//! @brief Load the archive header
@@ -535,6 +550,10 @@ class ario
535550
return {};
536551
}
537552

553+
//------------------------------------------------------------------------------
554+
//! @brief Save the archive header
555+
//! @param os Output stream to save the header
556+
//! @return Error object indicating success or failure
538557
Result save_header( std::ostream& os )
539558
{
540559
if ( !os ) {
@@ -548,6 +567,11 @@ class ario
548567
}
549568

550569
//------------------------------------------------------------------------------
570+
//! @brief Calculate the size of the symbol table
571+
//! @return The size of the symbol table in bytes
572+
//! The size includes the header, number of symbols, symbol locations, and names
573+
//! It also includes padding if the size is odd
574+
//! @note The symbol table is saved after the archive header and before the long name directory
551575
std::streamoff calculate_symbol_table_size() const
552576
{
553577
auto num_of_symbols = static_cast<uint32_t>( symbol_table.size() );
@@ -569,6 +593,10 @@ class ario
569593
return symbol_table_size;
570594
}
571595

596+
//------------------------------------------------------------------------------
597+
//! @brief Calculate the relative offsets of members in the archive
598+
//! @return A vector of relative offsets for each member
599+
//! The offsets are calculated from the start of the archive
572600
std::vector<uint32_t> calculate_member_relative_offsets()
573601
{
574602
std::vector<uint32_t> member_relative_offset;
@@ -585,6 +613,12 @@ class ario
585613
return member_relative_offset;
586614
}
587615

616+
//------------------------------------------------------------------------------
617+
//! @brief Calculate the size of the long names directory
618+
//! @return The size of the long names directory in bytes
619+
//! The size includes the header and the string table
620+
//! It also includes padding if the size is odd
621+
//! @note The long names directory is saved after the symbol table and before the members
588622
std::streamoff calculate_long_names_dir_size()
589623
{
590624
return HEADER_SIZE + string_table.size() + string_table.size() % 2;

‎examples/archive_dump/archive_dump.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ int main( int argc, char** argv )
4343
std::cout << "Member: " << std::setw( 40 ) << std::left << member.name
4444
<< " Size: " << std::setw( 8 ) << std::right << member.size
4545
<< " Mode: " << std::setw( 3 ) << std::oct << member.mode
46-
<< " Data: " << member.data().substr( 1, 3 ) << std::endl;
46+
<< std::endl;
4747
std::vector<std::string> symbols;
4848
result = archive.get_symbols_for_member( member, symbols );
4949
if ( result.ok() ) {

‎tests/ARIOTest.cpp‎

Lines changed: 47 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -132,23 +132,24 @@ TEST( ARIOTest, find_symbol_libgcov )
132132
ario archive;
133133
ASSERT_EQ( archive.load( "ario/libgcov.a" ).ok(), true );
134134

135-
ario::Member member;
136-
auto result = archive.find_symbol( "__gcov_merge_add", member );
135+
std::optional<std::reference_wrapper<const ario::Member>> member =
136+
std::nullopt;
137+
auto result = archive.find_symbol( "__gcov_merge_add", member );
137138
ASSERT_EQ( result.ok(), true );
138-
ASSERT_EQ( member.name, "_gcov_merge_add.o" );
139+
ASSERT_EQ( member->get().name, "_gcov_merge_add.o" );
139140

140141
result =
141142
archive.find_symbol( "__gcov_indirect_call_topn_profiler", member );
142143
ASSERT_EQ( result.ok(), true );
143-
ASSERT_EQ( member.name, "_gcov_indirect_call_topn_profiler.o" );
144+
ASSERT_EQ( member->get().name, "_gcov_indirect_call_topn_profiler.o" );
144145

145146
result = archive.find_symbol( "__not_found", member );
146147
ASSERT_EQ( result.ok(), false );
147-
ASSERT_EQ( member.name, "_gcov_indirect_call_topn_profiler.o" );
148+
ASSERT_EQ( member.has_value(), false );
148149

149150
result = archive.find_symbol( "__gcov_write_counter", member );
150151
ASSERT_EQ( result.ok(), true );
151-
ASSERT_EQ( member.name, "_gcov.o" );
152+
ASSERT_EQ( member->get().name, "_gcov.o" );
152153
}
153154

154155
////////////////////////////////////////////////////////////////////////////////
@@ -218,7 +219,8 @@ TEST( ARIOTest, get_symbols_for_ELF_files_in_archive )
218219
Elf_Half section_index;
219220
unsigned char other;
220221

221-
ario::Member found_member;
222+
std::optional<std::reference_wrapper<const ario::Member>>
223+
found_member = std::nullopt;
222224
// Iterate over all symbols in the symbol table
223225
for ( Elf_Xword i = 0; i < symbols.get_symbols_num(); ++i ) {
224226
// Extract symbol properties
@@ -330,31 +332,33 @@ TEST( ARIOTest, add_simple_member )
330332
ario archive;
331333
ASSERT_EQ( archive.load( "ario/simple_text.a" ).ok(), true );
332334

333-
ario::Member m;
335+
ario::Member m;
336+
std::optional<std::reference_wrapper<const ario::Member>> added_member =
337+
std::nullopt;
334338
m.name = "added_text.txt";
335339
m.date = 0;
336340
m.gid = 1234;
337341
m.uid = 5678;
338342
m.mode = 0644;
339-
archive.add_member( m, "The content\nof this\nmember" );
343+
archive.add_member( m, "The content\nof this\nmember", added_member );
340344
m.name = "added_text1.txt";
341345
m.date = 0;
342346
m.gid = 1234;
343347
m.uid = 5678;
344348
m.mode = 0644;
345-
archive.add_member( m, "The content\nof this\nmember\n" );
349+
archive.add_member( m, "The content\nof this\nmember\n", added_member );
346350
m.name = "added_text2.txt";
347351
m.date = 0;
348352
m.gid = 1234;
349353
m.uid = 5678;
350354
m.mode = 0644;
351-
archive.add_member( m, "" );
355+
archive.add_member( m, "", added_member );
352356
m.name = "added_text3.txt";
353357
m.date = 0;
354358
m.gid = 1234;
355359
m.uid = 5678;
356360
m.mode = 0644;
357-
archive.add_member( m, "Hello\n" );
361+
archive.add_member( m, "Hello\n", added_member );
358362

359363
// Save the archive to a new file
360364
auto result = archive.save( "ario/simple_text_saved.a" );
@@ -395,31 +399,33 @@ TEST( ARIOTest, add_long_name_member )
395399
ario archive;
396400
ASSERT_EQ( archive.load( "ario/simple_text.a" ).ok(), true );
397401

398-
ario::Member m;
402+
ario::Member m;
403+
std::optional<std::reference_wrapper<const ario::Member>> added_member =
404+
std::nullopt;
399405
m.name = "long_name_member_added_text.txt";
400406
m.date = 0;
401407
m.gid = 1234;
402408
m.uid = 5678;
403409
m.mode = 0644;
404-
archive.add_member( m, "The content\nof this\nmember" );
410+
archive.add_member( m, "The content\nof this\nmember", added_member );
405411
m.name = "long_name_member_added_text1.txt";
406412
m.date = 0;
407413
m.gid = 1234;
408414
m.uid = 5678;
409415
m.mode = 0644;
410-
archive.add_member( m, "The content\nof this\nmember\n" );
416+
archive.add_member( m, "The content\nof this\nmember\n", added_member );
411417
m.name = "long_name_member_added_text2.txt";
412418
m.date = 0;
413419
m.gid = 1234;
414420
m.uid = 5678;
415421
m.mode = 0644;
416-
archive.add_member( m, "" );
422+
archive.add_member( m, "", added_member );
417423
m.name = "long_name_member_added_text333.txt";
418424
m.date = 0;
419425
m.gid = 1234;
420426
m.uid = 5678;
421427
m.mode = 0644;
422-
archive.add_member( m, "Hello\n" );
428+
archive.add_member( m, "Hello\n", added_member );
423429

424430
// Save the archive to a new file
425431
auto result = archive.save( "ario/long_name_saved.a" );
@@ -459,43 +465,45 @@ TEST( ARIOTest, new_text_lib )
459465
{
460466
ario archive;
461467

462-
ario::Member m;
468+
ario::Member m;
469+
std::optional<std::reference_wrapper<const ario::Member>> added_member =
470+
std::nullopt;
463471
m.name = "123456789012345";
464472
m.date = 0;
465473
m.gid = 1234;
466474
m.uid = 5678;
467475
m.mode = 0644;
468-
archive.add_member( m, "data\n" );
476+
archive.add_member( m, "data\n", added_member );
469477
m.name = "1234567890123456";
470478
m.date = 0;
471479
m.gid = 1234;
472480
m.uid = 5678;
473481
m.mode = 0644;
474-
archive.add_member( m, "data\n" );
482+
archive.add_member( m, "data\n", added_member );
475483
m.name = "12345678901234567";
476484
m.date = 0;
477485
m.gid = 1234;
478486
m.uid = 5678;
479487
m.mode = 0644;
480-
archive.add_member( m, "data\n" );
488+
archive.add_member( m, "data\n", added_member );
481489
m.name = "12345";
482490
m.date = 0;
483491
m.gid = 1234;
484492
m.uid = 5678;
485493
m.mode = 0644;
486-
archive.add_member( m, "data\n" );
494+
archive.add_member( m, "data\n", added_member );
487495
m.name = "123456789012345678";
488496
m.date = 0;
489497
m.gid = 1234;
490498
m.uid = 5678;
491499
m.mode = 0644;
492-
archive.add_member( m, "data\n" );
500+
archive.add_member( m, "data\n", added_member );
493501
m.name = "1234567";
494502
m.date = 0;
495503
m.gid = 1234;
496504
m.uid = 5678;
497505
m.mode = 0644;
498-
archive.add_member( m, "data\n" );
506+
archive.add_member( m, "data\n", added_member );
499507

500508
// Save the archive to a new file
501509
auto result = archive.save( "ario/new_text_lib.a" );
@@ -521,21 +529,27 @@ TEST( ARIOTest, new_text_lib_with_symbols )
521529
ario archive;
522530

523531
for ( auto i = 0; i < 20; i++ ) {
524-
ario::Member m;
532+
ario::Member m;
533+
std::optional<std::reference_wrapper<const ario::Member>> added_member =
534+
std::nullopt;
525535
m.name = "name__________" + std::to_string( i );
526536
m.date = 0;
527537
m.gid = 1234;
528538
m.uid = 5678;
529539
m.mode = 0644;
530-
archive.add_member( m, "data" + std::to_string( i ) + "\n" );
540+
ASSERT_EQ( archive
541+
.add_member( m, "data" + std::to_string( i ) + "\n",
542+
added_member )
543+
.ok(),
544+
true );
531545

532-
const auto& added_member = archive.members[m.name];
533-
std::vector<std::string> symbols = {};
546+
std::vector<std::string> symbols = {};
534547
for ( auto j = 0; j < i; j++ ) {
535548
symbols.emplace_back( "symbol_" + std::to_string( 100 * i + j ) );
536549
}
537-
ASSERT_EQ( archive.add_symbols_for_member( added_member, symbols ).ok(),
538-
true );
550+
ASSERT_EQ(
551+
archive.add_symbols_for_member( added_member->get(), symbols ).ok(),
552+
true );
539553
}
540554
// Save the archive to a new file
541555
auto result = archive.save( "ario/new_text_lib_with_symbols.a" );
@@ -555,9 +569,10 @@ TEST( ARIOTest, new_text_lib_with_symbols )
555569
true );
556570
ASSERT_EQ( symbols.size(), index );
557571
for ( const auto& symbol : symbols ) {
558-
ario::Member ms;
572+
std::optional<std::reference_wrapper<const ario::Member>> ms =
573+
std::nullopt;
559574
ASSERT_EQ( loaded_archive.find_symbol( symbol, ms ).ok(), true );
560-
ASSERT_EQ( ms.name, m.name );
575+
ASSERT_EQ( ms->get().name, m.name );
561576
}
562577
}
563578
}

0 commit comments

Comments
 (0)