Hi all, the attached patch is a direct c++ refactoring and documentation effort for struct fileMap (which gets renamed class FileMap). Scope is very limited; only ufs uses this code. Main benefit is the reduction in pollution in global include files.
-- /kinkie
=== added file 'src/FileMap.h' --- src/FileMap.h 1970-01-01 00:00:00 +0000 +++ src/FileMap.h 2011-11-23 22:11:09 +0000 @@ -0,0 +1,99 @@ +/* + * FileMap.h + * + * DEBUG: section 08 Swap File Bitmap + * AUTHOR: Harvest Derived + * + * SQUID Web Proxy Cache http://www.squid-cache.org/ + * ---------------------------------------------------------- + * + * Squid is the result of efforts by numerous individuals from + * the Internet community; see the CONTRIBUTORS file for full + * details. Many organizations have provided support for Squid's + * development; see the SPONSORS file for full details. Squid is + * Copyrighted (C) 2001 by the Regents of the University of + * California; see the COPYRIGHT file for full details. Squid + * incorporates software developed and/or copyrighted by other + * sources; see the CREDITS file for full details. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111, USA. + */ + +#ifndef FILEMAP_H_ +#define FILEMAP_H_ + +/** Compact bit-field representation class + * + * FileMap is aimed at holding UFS's internal file allocation table. + * A FileMap is at most 2^24 slots big, and is automatically extended + * as needed up to that size. The slots' initial state is "empty". + */ +class FileMap +{ +public: + FileMap(); + ~FileMap(); + + /** Set the num-th bit in the FileMap + * + * The FileMap's backing storage will be extended as needed to + * hold the representation, but if the bit is already set + * it will break the file number accounting, so the caller must + * ensure that setBit is only called if the bit is not already set, + * by using testBit on it before. + */ + + int setBit(int num); + + /// Test whether the num-th bit in the FileMap is set + int testBit(int num) const; + + /** Clear the num-th bit in the FileMap + * + * Notice that clearBit doesn't do any bounds checking, nor it + * checks that the bit is set before clearing. The caller will have + * to ensure that both are true using testBit before clearBit. + */ + void clearBit(int num); + + /** locate an unused slot in the FileMap, possibly at or after position suggestion + * + * Obtain the location of an unused slot in the FileMap, + * growing it if needed. + * The suggestion is only an advice; there is no guarantee + * that it will be followed. + * + */ + int allocate(int suggestion); + + /// return the max number of slots in the FileMap + int maxNFiles() const {return max_n_files;} + + /// return the number of used slots in the FileMap + int numFilesInMap() const {return n_files_in_map;} +private: + /// grow the FileMap (size is doubled each time, up to 2^24 bits) + void grow(); + FileMap(const FileMap &); //no copying + FileMap& operator=(const FileMap &); //no assignments + + // max number of files which can be tracked in the current store + int max_n_files; + int n_files_in_map; // used slots in the map + int nwords; // number of "long ints" making up the filemap + unsigned long *file_map; +}; + +#endif /* FILEMAP_H_ */ === modified file 'src/Makefile.am' --- src/Makefile.am 2011-11-04 12:30:43 +0000 +++ src/Makefile.am 2011-11-23 22:14:08 +0000 @@ -323,6 +323,7 @@ fd.cc \ fde.cc \ fde.h \ + FileMap.h \ filemap.cc \ forward.cc \ forward.h \ @@ -1134,6 +1135,7 @@ DiskIO/WriteRequest.cc \ ETag.cc \ event.cc \ + FileMap.h \ filemap.cc \ HelperChildConfig.h \ HelperChildConfig.cc \ @@ -1296,6 +1298,7 @@ ExternalACLEntry.cc \ fd.cc \ fde.cc \ + FileMap.h \ filemap.cc \ forward.cc \ fqdncache.cc \ @@ -1446,6 +1449,7 @@ EventLoop.cc \ event.cc \ fd.cc \ + FileMap.h \ filemap.cc \ HttpBody.cc \ HttpHdrCc.h \ @@ -1612,6 +1616,7 @@ FadingCounter.cc \ fd.cc \ fde.cc \ + FileMap.h \ filemap.cc \ forward.cc \ fqdncache.cc \ @@ -1800,6 +1805,7 @@ FadingCounter.cc \ fd.cc \ fde.cc \ + FileMap.h \ filemap.cc \ forward.cc \ fqdncache.cc \ @@ -1986,6 +1992,7 @@ FadingCounter.cc \ fd.cc \ fde.cc \ + FileMap.h \ filemap.cc \ forward.cc \ fqdncache.cc \ @@ -2361,6 +2368,7 @@ ETag.cc \ event.cc \ EventLoop.cc \ + FileMap.h \ filemap.cc \ HttpHdrCc.h \ HttpHdrCc.cc \ @@ -2550,6 +2558,7 @@ tests/stub_store_stats.cc \ fd.cc \ disk.cc \ + FileMap.h \ filemap.cc \ HttpBody.cc \ HttpReply.cc \ @@ -2679,6 +2688,7 @@ EventLoop.cc \ event.cc \ fd.cc \ + FileMap.h \ filemap.cc \ HttpBody.cc \ HttpHdrCc.cc \ @@ -2808,6 +2818,7 @@ tests/stub_store_stats.cc \ fd.cc \ disk.cc \ + FileMap.h \ filemap.cc \ HttpBody.cc \ HttpReply.cc \ @@ -2939,6 +2950,7 @@ tests/stub_store_stats.cc \ fd.cc \ disk.cc \ + FileMap.h \ filemap.cc \ HttpBody.cc \ HttpReply.cc \ @@ -3087,6 +3099,7 @@ ExternalACLEntry.cc \ fd.cc \ fde.cc \ + FileMap.h \ filemap.cc \ forward.cc \ fqdncache.cc \ === modified file 'src/filemap.cc' --- src/filemap.cc 2011-09-02 12:35:57 +0000 +++ src/filemap.cc 2011-11-23 22:11:12 +0000 @@ -1,6 +1,4 @@ /* - * $Id$ - * * DEBUG: section 08 Swap File Bitmap * AUTHOR: Harvest Derived * @@ -33,6 +31,7 @@ */ #include "squid.h" +#include "FileMap.h" /* Number of bits in a long */ #if SIZEOF_LONG == 8 @@ -54,133 +53,102 @@ #define FM_INITIAL_NUMBER (1<<14) -fileMap * -file_map_create(void) -{ - fileMap *fm = (fileMap *)xcalloc(1, sizeof(fileMap)); - fm->max_n_files = FM_INITIAL_NUMBER; - fm->nwords = fm->max_n_files >> LONG_BIT_SHIFT; - debugs(8, 3, "file_map_create: creating space for " << fm->max_n_files << " files"); - debugs(8, 5, "--> " << fm->nwords << " words of " << sizeof(*fm->file_map) << " bytes each"); - fm->file_map = (unsigned long *)xcalloc(fm->nwords, sizeof(*fm->file_map)); - /* XXX account fm->file_map */ - return fm; +FileMap::FileMap() { + max_n_files = FM_INITIAL_NUMBER; + nwords = max_n_files >> LONG_BIT_SHIFT; + debugs(8, 3, HERE << "creating space for " << max_n_files << " files"); + debugs(8, 5, "--> " << nwords << " words of " << sizeof(*file_map) << " bytes each"); + file_map = (unsigned long *)xcalloc(nwords, sizeof(*file_map)); } -static void -file_map_grow(fileMap * fm) +void +FileMap::grow() { - int old_sz = fm->nwords * sizeof(*fm->file_map); - void *old_map = fm->file_map; - fm->max_n_files <<= 1; - assert(fm->max_n_files <= (1 << 24)); /* swap_filen is 25 bits, signed */ - fm->nwords = fm->max_n_files >> LONG_BIT_SHIFT; - debugs(8, 3, "file_map_grow: creating space for " << fm->max_n_files << " files"); - debugs(8, 5, "--> " << fm->nwords << " words of " << sizeof(*fm->file_map) << " bytes each"); - fm->file_map = (unsigned long *)xcalloc(fm->nwords, sizeof(*fm->file_map)); + int old_sz = nwords * sizeof(*file_map); + void *old_map = file_map; + max_n_files <<= 1; + assert(max_n_files <= (1 << 24)); /* swap_filen is 25 bits, signed */ + nwords = max_n_files >> LONG_BIT_SHIFT; + debugs(8, 3, HERE << " creating space for " << max_n_files << " files"); + debugs(8, 5, "--> " << nwords << " words of " << sizeof(*file_map) << " bytes each"); + file_map = (unsigned long *)xcalloc(nwords, sizeof(*file_map)); debugs(8, 3, "copying " << old_sz << " old bytes"); - memcpy(fm->file_map, old_map, old_sz); + memcpy(file_map, old_map, old_sz); xfree(old_map); /* XXX account fm->file_map */ } int -file_map_bit_set(fileMap * fm, int file_number) +FileMap::setBit(int file_number) { unsigned long bitmask = (1L << (file_number & LONG_BIT_MASK)); - while (file_number >= fm->max_n_files) - file_map_grow(fm); - - fm->file_map[file_number >> LONG_BIT_SHIFT] |= bitmask; - - fm->n_files_in_map++; + while (file_number >= max_n_files) + grow(); + + file_map[file_number >> LONG_BIT_SHIFT] |= bitmask; + + n_files_in_map++; return file_number; } -/* - * WARNING: file_map_bit_reset does not perform array bounds - * checking! It assumes that 'file_number' is valid, and that the - * bit is already set. The caller must verify both of those - * conditions by calling file_map_bit_test() first. - */ void -file_map_bit_reset(fileMap * fm, int file_number) +FileMap::clearBit(int file_number) { unsigned long bitmask = (1L << (file_number & LONG_BIT_MASK)); - fm->file_map[file_number >> LONG_BIT_SHIFT] &= ~bitmask; - fm->n_files_in_map--; + file_map[file_number >> LONG_BIT_SHIFT] &= ~bitmask; + n_files_in_map--; } int -file_map_bit_test(fileMap * fm, int file_number) +FileMap::testBit(int file_number) const { unsigned long bitmask = (1L << (file_number & LONG_BIT_MASK)); - if (file_number >= fm->max_n_files) + if (file_number >= max_n_files) return 0; /* be sure the return value is an int, not a u_long */ - return (fm->file_map[file_number >> LONG_BIT_SHIFT] & bitmask ? 1 : 0); + return (file_map[file_number >> LONG_BIT_SHIFT] & bitmask ? 1 : 0); } int -file_map_allocate(fileMap * fm, int suggestion) +FileMap::allocate(int suggestion) { int word; int bit; int count; - if (suggestion >= fm->max_n_files) + if (suggestion >= max_n_files) suggestion = 0; - if (!file_map_bit_test(fm, suggestion)) + if (!testBit(suggestion)) return suggestion; word = suggestion >> LONG_BIT_SHIFT; - for (count = 0; count < fm->nwords; count++) { - if (fm->file_map[word] != ALL_ONES) + for (count = 0; count < nwords; count++) { + if (file_map[word] != ALL_ONES) break; - word = (word + 1) % fm->nwords; + word = (word + 1) % nwords; } for (bit = 0; bit < BITS_IN_A_LONG; bit++) { suggestion = ((unsigned long) word << LONG_BIT_SHIFT) | bit; - if (!file_map_bit_test(fm, suggestion)) { + if (!testBit(suggestion)) { return suggestion; } } - debugs(8, 3, "growing from file_map_allocate"); - file_map_grow(fm); - return file_map_allocate(fm, fm->max_n_files >> 1); -} - -void -filemapFreeMemory(fileMap * fm) -{ - safe_free(fm->file_map); - safe_free(fm); -} - -#ifdef TEST - -#define TEST_SIZE 1<<16 -main(argc, argv) -{ - int i; - - fm = file_map_create(TEST_SIZE); - - for (i = 0; i < TEST_SIZE; ++i) { - file_map_bit_set(i); - assert(file_map_bit_test(i)); - file_map_bit_reset(i); - } -} - -#endif + debugs(8, 3, HERE << "growing fileMap"); + grow(); + return allocate(max_n_files >> 1); +} + +FileMap::~FileMap() +{ + safe_free(file_map); +} === modified file 'src/fs/ufs/store_dir_ufs.cc' --- src/fs/ufs/store_dir_ufs.cc 2011-11-08 15:00:17 +0000 +++ src/fs/ufs/store_dir_ufs.cc 2011-11-22 22:05:28 +0000 @@ -41,6 +41,7 @@ #include "ConfigOption.h" #include "DiskIO/DiskIOStrategy.h" #include "DiskIO/DiskIOModule.h" +#include "FileMap.h" #include "Parsing.h" #include "SquidMath.h" #include "SquidTime.h" @@ -246,7 +247,7 @@ createSwapSubDirs(); } -UFSSwapDir::UFSSwapDir(char const *aType, const char *anIOType) : SwapDir(aType), IO(NULL), map(file_map_create()), suggest(0), swaplog_fd (-1), currentIOOptions(new ConfigOptionVector()), ioType(xstrdup(anIOType)), cur_size(0), n_disk_objects(0) +UFSSwapDir::UFSSwapDir(char const *aType, const char *anIOType) : SwapDir(aType), IO(NULL), map(new FileMap()), suggest(0), swaplog_fd (-1), currentIOOptions(new ConfigOptionVector()), ioType(xstrdup(anIOType)), cur_size(0), n_disk_objects(0) { /* modulename is only set to disk modules that are built, by configure, * so the Find call should never return NULL here. @@ -261,7 +262,7 @@ swaplog_fd = -1; } - filemapFreeMemory(map); + delete map; if (IO) delete IO; @@ -321,8 +322,8 @@ storeAppendPrintf(&sentry, "Percent Used: %0.2f%%\n", Math::doublePercent(currentSize(), maxSize())); storeAppendPrintf(&sentry, "Filemap bits in use: %d of %d (%d%%)\n", - map->n_files_in_map, map->max_n_files, - Math::intPercent(map->n_files_in_map, map->max_n_files)); + map->numFilesInMap(), map->maxNFiles(), + Math::intPercent(map->numFilesInMap(), map->maxNFiles())); x = storeDirGetUFSStats(path, &totl_kb, &free_kb, &totl_in, &free_in); if (0 == x) { @@ -450,36 +451,36 @@ int UFSSwapDir::mapBitTest(sfileno filn) { - return file_map_bit_test(map, filn); + return map->testBit(filn); } void UFSSwapDir::mapBitSet(sfileno filn) { - file_map_bit_set(map, filn); + map->setBit(filn); } void UFSSwapDir::mapBitReset(sfileno filn) { /* - * We have to test the bit before calling file_map_bit_reset. - * file_map_bit_reset doesn't do bounds checking. It assumes + * We have to test the bit before calling resetting it as + * resetting doesn't do bounds checking. It assumes * filn is a valid file number, but it might not be because * the map is dynamic in size. Also clearing an already clear * bit puts the map counter of-of-whack. */ - if (file_map_bit_test(map, filn)) - file_map_bit_reset(map, filn); + if (map->testBit(filn)) + map->clearBit(filn); } int UFSSwapDir::mapBitAllocate() { int fn; - fn = file_map_allocate(map, suggest); - file_map_bit_set(map, fn); + fn = map->allocate(suggest); + map->setBit(fn); suggest = fn + 1; return fn; } @@ -1262,7 +1263,7 @@ * be considered invalid. */ if (flag) - if (filn > map->max_n_files) + if (filn > map->maxNFiles()) return 0; return 1; === modified file 'src/fs/ufs/ufscommon.h' --- src/fs/ufs/ufscommon.h 2011-10-27 23:14:28 +0000 +++ src/fs/ufs/ufscommon.h 2011-11-21 23:16:35 +0000 @@ -40,6 +40,7 @@ class ConfigOptionVector; class DiskIOModule; class StoreSearch; +class FileMap; #include "SwapDir.h" @@ -116,7 +117,7 @@ void replacementRemove(StoreEntry *e); protected: - fileMap *map; + FileMap *map; int suggest; int l1; int l2; === modified file 'src/ipc/StoreMap.h' --- src/ipc/StoreMap.h 2011-10-28 01:01:41 +0000 +++ src/ipc/StoreMap.h 2011-11-21 22:55:27 +0000 @@ -195,7 +195,7 @@ } // namespace Ipc -// We do not reuse struct _fileMap because we cannot control its size, +// We do not reuse sileMap because we cannot control its size, // resulting in sfilenos that are pointing beyond the database. #endif /* SQUID_IPC_STORE_MAP_H */ === modified file 'src/protos.h' --- src/protos.h 2011-11-03 10:02:02 +0000 +++ src/protos.h 2011-11-21 22:55:27 +0000 @@ -144,14 +144,6 @@ SQUIDCEXTERN int fdUsageHigh(void); SQUIDCEXTERN void fdAdjustReserved(void); -SQUIDCEXTERN fileMap *file_map_create(void); -SQUIDCEXTERN int file_map_allocate(fileMap *, int); -SQUIDCEXTERN int file_map_bit_set(fileMap *, int); -SQUIDCEXTERN int file_map_bit_test(fileMap *, int); -SQUIDCEXTERN void file_map_bit_reset(fileMap *, int); -SQUIDCEXTERN void filemapFreeMemory(fileMap *); - - SQUIDCEXTERN void fqdncache_nbgethostbyaddr(const Ip::Address &, FQDNH *, void *); SQUIDCEXTERN const char *fqdncache_gethostbyaddr(const Ip::Address &, int flags); === modified file 'src/structs.h' --- src/structs.h 2011-11-08 10:54:37 +0000 +++ src/structs.h 2011-11-20 18:04:38 +0000 @@ -696,14 +696,6 @@ off_t offset; }; -struct _fileMap { - int max_n_files; - int n_files_in_map; - int toggle; - int nwords; - unsigned long *file_map; -}; - /* * Note: HttpBody is used only for messages with a small content that is * known a priory (e.g., error messages). === modified file 'src/typedefs.h' --- src/typedefs.h 2011-07-25 13:10:17 +0000 +++ src/typedefs.h 2011-11-21 22:55:27 +0000 @@ -56,8 +56,6 @@ typedef struct _dwrite_q dwrite_q; -typedef struct _fileMap fileMap; - typedef struct _HttpHeaderFieldAttrs HttpHeaderFieldAttrs; typedef struct _HttpHeaderStat HttpHeaderStat;
