commit 57e6c22f916073f4ac9c4653db920f8312dd46a2 Author: David Sorber Date: Tue Feb 21 17:19:50 2017 -0500 Finished up the changes to make argv a std::vector instead of a regular array. I also implemented the changes to control the size of argv based on system configuration and potentially spawn packJPG multiple times in order to handle a very large number of files. I ran a test on my Photos library and it sort of worked but I ran into some issues that I still need to diagnose. diff --git a/software/photo_compress_archiver/PhotoCompressArchiver.cc b/software/photo_compress_archiver/PhotoCompressArchiver.cc index 7bacfd1..36af5ec 100644 --- a/software/photo_compress_archiver/PhotoCompressArchiver.cc +++ b/software/photo_compress_archiver/PhotoCompressArchiver.cc @@ -20,6 +20,7 @@ const char *PROG_OPT2 = "-o"; const char *PROG_OPT3 = "-p"; const uint32_t ARGV_MAX_SIZE(sysconf(_SC_ARG_MAX)); +const uint32_t ARGV_EXTRA(4096); PhotoCompressArchiver::PhotoCompressArchiver( const std::string& path, @@ -42,8 +43,9 @@ PhotoCompressArchiver::PhotoCompressArchiver( // This is a somewhat crude way to determining if the filter regex is empty m_filter_empty = std::string("").compare(filter_regex.str()) == 0; + + // Remove this... std::cout << "argv size: " << sysconf(_SC_ARG_MAX) << std::endl; - std::cout << "appname size: " << sizeof(PROG_NAME) << std::endl; } PhotoCompressArchiver::~PhotoCompressArchiver() @@ -60,6 +62,15 @@ int PhotoCompressArchiver::execute() p_finder_thread->join(); OUT(std::cout << "File finder completed:" << std::endl;) + // Error out of no input files were found + if (m_num_input_files == 0) + { + OUT(std::cerr << BOLD << RED << "ERROR: " << ENDC << "No matching " + << "input files found. Please check the patch and try " + << "again. " << std::endl;) + return -1; + } + OUT(std::cout << " Found " << m_num_input_files << " images" << std::endl;) if (! m_decompress) @@ -73,7 +84,7 @@ int PhotoCompressArchiver::execute() // TODO: need to handle the case were number of input files is less than // the number of threads - uint32_t num_threads = 1;//m_num_cores; + uint32_t num_threads = m_num_cores; uint32_t sublist_len = m_num_input_files / num_threads; for (uint32_t idx = 0; idx < num_threads; ++idx) @@ -194,150 +205,188 @@ void PhotoCompressArchiver::fork_worker( OUT(std::cout << "T[" << tid << "] file: " << filepath.string() << std::endl;) } + + return; // temporary for debugging #endif - //~ return; // temporary for debugging - - //~ uint32_t argv_size = file_sublist->size() + 5; - //~ const char **exec_argv = new const char* [argv_size]; + uint32_t sublist_idx = 0; + const uint32_t ptr_len = sizeof(char*); + int status; - // Build the argv array; pass in default program options + // Vector of pointers that will become the char** argv passed to packjpg std::vector exec_argv; - exec_argv.push_back(&PROG_NAME[2]); - exec_argv.push_back(PROG_OPT1); - exec_argv.push_back(PROG_OPT2); - exec_argv.push_back(PROG_OPT3); - - // Build the argv array; pass in default program options - //~ exec_argv[0] = &PROG_NAME[2]; // discard leading "./" - //~ exec_argv[1] = PROG_OPT1; - //~ exec_argv[2] = PROG_OPT2; - //~ exec_argv[3] = PROG_OPT3; - - uint32_t argv_size = sizeof(PROG_NAME) + sizeof(PROG_OPT1) + - sizeof(PROG_OPT2) + sizeof(PROG_OPT3) + 4096; - - // Add the sublist files to the argv array - uint32_t idx = 0; - for (auto& image_path : *file_sublist) - { - uint32_t image_path_len = image_path.string().size(); - if (argv_size + image_path_len < ARGV_MAX_SIZE) - { - exec_argv.push_back(image_path.string().c_str()); - argv_size += image_path_len + sizeof(char*); - ++idx; - } - else - { - break; - } - - //~ exec_argv[idx++] = image_path.string().c_str(); - } - //~ exec_argv[argv_size - 1] = nullptr; - exec_argv.push_back(nullptr); - - std::cout << "IDX: " << idx << std::endl; - // Build a list of the output files names for use in output file monitoring // below. Output file names have ".jpg" or ".jpeg" replaced with ".pjg" // in compression mode and vice versa in decompression mode. std::vector output_filenames; - for (auto& filepath : *file_sublist) - { - // Create new filename - uint32_t end_pos = filepath.string().find("."); - std::string* filename = new std::string(filepath.string().begin(), - filepath.string().begin() + end_pos); - - // Add extension depending on if decompress mode is enabled - (*filename) += (m_decompress ? ".jpg" : ".pjg"); - - output_filenames.push_back(filename); - } - // Create a pipe to hold stdout from child process - int filedes[2]; - if (pipe(filedes) == -1) + while (sublist_idx < sublist_file_count) { - perror("pipe"); - exit(1); - } + // Pass in default program options for packjpg + exec_argv.clear(); + exec_argv.push_back(&PROG_NAME[2]); + exec_argv.push_back(PROG_OPT1); + exec_argv.push_back(PROG_OPT2); + exec_argv.push_back(PROG_OPT3); - // Fork off the childprocess - //~ pid_t parent = getpid(); - pid_t pid = vfork(); - - if (pid == -1) - { - // error, failed to fork() - OUT(std::cout << "T[" << tid << "] fork failed!" << std::endl;) - } - else if (pid == 0) - { - // This is the child process... - // Set child process's stdout to the pipe entry - //~ while ((dup2(filedes[1], STDOUT_FILENO) == -1) && (errno == EINTR)) {} - //~ close(filedes[1]); - //~ close(filedes[0]); + uint32_t argv_size = sizeof(PROG_NAME) + sizeof(PROG_OPT1) + + sizeof(PROG_OPT2) + sizeof(PROG_OPT3) + ARGV_EXTRA; + + output_filenames.clear(); - // Child after fork - int rc = execv(PROG_NAME, (char **)exec_argv.data()); - if (rc) + while (sublist_idx < sublist_file_count) { - std::cerr << "execv failed: " << errno << std::endl; + bfs::path& file_path = file_sublist->at(sublist_idx); + uint32_t path_len = file_path.string().size(); + + // Check if adding this next file would make the argv too big; if + // so bail out + if (argv_size + path_len + ptr_len > ARGV_MAX_SIZE) + { + break; + } + + // Add the file to the argv array + exec_argv.push_back(file_path.string().c_str()); + argv_size += path_len + ptr_len; + + // Create new filename for expected + uint32_t end_pos = file_path.string().find("."); + std::string* filename = new std::string(file_path.string().begin(), + file_path.string().begin() + + end_pos); + + // Add extension depending on if decompress mode is enabled or not + (*filename) += (m_decompress ? ".jpg" : ".pjg"); + output_filenames.push_back(filename); + + + // Increment our index to the next file + ++sublist_idx; } - _exit(EXIT_FAILURE); // exec never returns - } - - // This is the parent process... - - // This is a little silly... but it works. We know the order in which the - // output files will be created so we wait until output files X + 1 exists - // (i.e. we can stat() it) and then process file X. - const uint32_t BAILOUT_MAX = 600; // 30s in 50ms increments - uint32_t bailout_ctr = 0; - uint32_t output_idx = 1; - int stat_rc = 0; - struct stat statbuf; - double complete_percent = 0.0; - while (output_idx < output_filenames.size()) - { - // Poll, waiting for output file X + 1 to exist - bailout_ctr = 0; - while ((stat(output_filenames[output_idx]->c_str(), &statbuf) != 0) && - (++bailout_ctr < BAILOUT_MAX)) + + // Add a nullptr to the end of the argv array as a terminator + exec_argv.push_back(nullptr); + + // Create a pipe to hold stdout from child process + int filedes[2]; + if (pipe(filedes) == -1) { - std::this_thread::sleep_for(std::chrono::milliseconds(50)); + perror("pipe"); + exit(1); } - // Check if timeout has occurred - if (bailout_ctr == BAILOUT_MAX) + // Fork off the childprocess + //~ pid_t parent = getpid(); + pid_t pid = vfork(); + if (pid == -1) { - OUT(std::cerr << "T[" << tid << "] " << BOLD << RED << "ERROR: " - << ENDC << "timed out while waiting " - << "for output file: " - << *output_filenames[output_idx] << "; aborting" - << std::endl;) - - int status; - waitpid(pid, &status, 0); - - OUT(std::cerr << "T[" << tid << "] " << BOLD << RED << "ERROR: " - << ENDC << PURPLE << " RC => " << status << ENDC - << std::endl;) + // error, failed to fork() + OUT(std::cout << "T[" << tid << "] fork failed!" << std::endl;) + } + else if (pid == 0) + { + // This is the child process... + // Set child process's stdout to the pipe entry + //~ while ((dup2(filedes[1], STDOUT_FILENO) == -1) && (errno == EINTR)) {} + //~ close(filedes[1]); + //~ close(filedes[0]); - return; + // Child after fork + int rc = execv(PROG_NAME, (char **)exec_argv.data()); + if (rc) + { + std::cerr << "execv failed: " << errno << std::endl; + } + _exit(EXIT_FAILURE); // exec never returns } + + // This is the parent process... + + // This is a little silly... but it works. We know the order in which + // the output files will be created so we wait until output files X + 1 + // exists (i.e. we can stat() it) and then process file X. + const uint32_t BAILOUT_MAX = 600; // 30s in 50ms increments + uint32_t bailout_ctr = 0; + uint32_t output_idx = 1; + int stat_rc = 0; + struct stat statbuf; + double complete_percent = 0.0; + while (output_idx < output_filenames.size()) + { + // Poll, waiting for output file X + 1 to exist + bailout_ctr = 0; + while ((stat(output_filenames[output_idx]->c_str(), &statbuf) != 0) && + (++bailout_ctr < BAILOUT_MAX)) + { + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + } + + // Check if timeout has occurred + if (bailout_ctr == BAILOUT_MAX) + { + OUT(std::cerr << "T[" << tid << "] " << BOLD << RED << "ERROR: " + << ENDC << "timed out while waiting " + << "for output file: " + << *output_filenames[output_idx] << "; aborting" + << std::endl;) + + // The subprocess timed out... get its status + waitpid(pid, &status, 0); + OUT(std::cerr << "T[" << tid << "] " << BOLD << RED << "ERROR: " + << ENDC << PURPLE << " RC => " << status << ENDC + << std::endl;) - // Now stat output file X which should exist + return; + } + + // Now stat output file X which should exist + stat_rc = stat(output_filenames[output_idx - 1]->c_str(), &statbuf); + if (stat_rc) + { + OUT(std::cerr << "T[" << tid << "] " << BOLD << RED << "ERROR " + << ENDC << "while stating: " + << *output_filenames[output_idx - 1] << std::endl;) + } + + // Increment counts + m_total_compressed_size += statbuf.st_size; + ++m_files_processed; + ++files_processed; + + // Unless keeping original files, remove the source file + if (! m_keep_orig) + { + std::remove(file_sublist->at(output_idx - 1).string().c_str()); + } + + // Calculate the local percent done + complete_percent = (double)files_processed / sublist_file_count; + complete_percent *= 100; + + OUT(std::cout << "T[" << tid << "] " << std::setw(6) + << std::fixed << std::setprecision(2) + << get_global_percent_done() + << "% -- (" << files_processed << "/" + << sublist_file_count << " -- " << std::setw(6) + << complete_percent + << "%) file: " << *output_filenames[output_idx - 1] + << " -- " << statbuf.st_size << std::endl;) + + ++output_idx; + } + + // Wait for forked child process to terminate + waitpid(pid, &status, 0); + + // Now that the forked process has completed, we can handle the last + // output file stat_rc = stat(output_filenames[output_idx - 1]->c_str(), &statbuf); if (stat_rc) { - OUT(std::cerr << BOLD << RED << "ERROR " << ENDC - << "while stating: " + OUT(std::cerr << "T[" << tid << "] " << BOLD << RED << "ERROR " + << ENDC << "while stating: " << *output_filenames[output_idx - 1] << std::endl;) } @@ -356,62 +405,22 @@ void PhotoCompressArchiver::fork_worker( complete_percent = (double)files_processed / sublist_file_count; complete_percent *= 100; - OUT(std::cout << "T[" << tid << "] " << std::setw(6) - << std::fixed << std::setprecision(2) - << get_global_percent_done() - << "% -- (" << files_processed << "/" - << sublist_file_count << " -- " << std::setw(6) - << complete_percent + OUT(std::cout << "T[" << tid << "] " << std::setw(6) << std::fixed + << std::setprecision(2) << get_global_percent_done() + << "% -- (" << files_processed << "/" << sublist_file_count + << " -- " << std::setw(5) << complete_percent << "%) file: " << *output_filenames[output_idx - 1] << " -- " << statbuf.st_size << std::endl;) - ++output_idx; - } - - // Wait for forked child process to terminate - int status; - waitpid(pid, &status, 0); - - // Now that the forked process has completed, we can handle the last output - // file - stat_rc = stat(output_filenames[output_idx - 1]->c_str(), &statbuf); - if (stat_rc) - { - OUT(std::cerr << BOLD << RED << "ERROR " << ENDC << "while stating: " - << *output_filenames[output_idx - 1] << std::endl;) - } - - // Increment counts - m_total_compressed_size += statbuf.st_size; - ++m_files_processed; - ++files_processed; - - // Unless keeping original files, remove the source file - if (! m_keep_orig) - { - std::remove(file_sublist->at(output_idx - 1).string().c_str()); + // Free up the output filename list + for (auto output_filename : output_filenames) + { + delete output_filename; + } } - // Calculate the local percent done - complete_percent = (double)files_processed / sublist_file_count; - complete_percent *= 100; - - OUT(std::cout << "T[" << tid << "] " << std::setw(6) << std::fixed - << std::setprecision(2) << get_global_percent_done() - << "% -- (" << files_processed << "/" << sublist_file_count - << " -- " << std::setw(5) << complete_percent << "%) file: " - << *output_filenames[output_idx - 1] << " -- " - << statbuf.st_size << std::endl;) - - OUT(std::cout << "T[" << tid << "] "<< PURPLE << "complete ==> RC: " + OUT(std::cout << "T[" << tid << "] " << PURPLE << "complete ==> RC: " << status << ENDC << std::endl;) - - // Free up the argv array and output filename list - //~ delete[] exec_argv; - for (auto output_filename : output_filenames) - { - delete output_filename; - } } double PhotoCompressArchiver::get_global_percent_done() diff --git a/software/photo_compress_archiver/main.cc b/software/photo_compress_archiver/main.cc index 999db6d..34559c0 100644 --- a/software/photo_compress_archiver/main.cc +++ b/software/photo_compress_archiver/main.cc @@ -39,7 +39,7 @@ void usage(char** argv) std::cout << std::endl; } - +// NOTE: useful filter: -f ^\\._.+$ int main(int argc, char** argv) { bpo::variables_map general_opts_vm; @@ -129,6 +129,8 @@ int main(int argc, char** argv) std::cerr << " " << e.what() << std::endl; return -1; } + + std::cout << " filter regex => ON (" << filter_regex << ")\n"; } std::cout << std::endl; diff --git a/software/photo_compress_archiver/packJPG/CMakeLists.txt b/software/photo_compress_archiver/packJPG/CMakeLists.txt index 7ab800a..a6f879b 100644 --- a/software/photo_compress_archiver/packJPG/CMakeLists.txt +++ b/software/photo_compress_archiver/packJPG/CMakeLists.txt @@ -9,7 +9,8 @@ set(packjpg_sources ) add_definitions("-std=c++1y -O3 -Wall -pedantic -DUNIX") -add_definitions("-funroll-loops -ffast-math -fsched-spec-load -fomit-frame-pointer") +add_definitions("-funroll-loops -ffast-math -fomit-frame-pointer") +#~ -fsched-spec-load #~ add_definitions("-static -static-libgcc -static-libstdc++") add_executable(packjpg diff --git a/software/photo_compress_archiver/packJPG/bitops.cpp b/software/photo_compress_archiver/packJPG/bitops.cpp index 7fcfad8..d4fa8dd 100644 --- a/software/photo_compress_archiver/packJPG/bitops.cpp +++ b/software/photo_compress_archiver/packJPG/bitops.cpp @@ -7,7 +7,7 @@ reading and writing of arrays #include #include -#include +#include #include #include