December 2017 Code Clean up

List the things that we want to do after the end of the semester during the code clean-up week:

Things we can do now

• Make everything enum class [Andy]
• Network Layer clean-up [Tianyu, Tianyi]
• Unify logging thread pool and worker thread pool [Tianyu]
  • Redesign thread pool class and eliminate one of the two different classes in the code base now [Tianyu]
• Remove redundant Transaction pointer argument for compiled plans; can be obtained from provided ExecutorContext argument [Pooja]
• Figure out what we want to do about settings/settings.cpp [Rohit]
• Remove this LOG_DEBUG line : https://github.com/cmu-db/peloton/blob/ffeb0819fc032d2444bd933563cf4ebbeb138ba1/test/executor/copy_test.cpp#L91 [Rohit]
• Make all public data members private
  • Planner [Bowei]
  • Networking [Team T]
• Figure out how to enforce that all data members are not public [Siva]
• Make sure that all important variables are clearly defined and explained. [Everyone]
  • Mention to this to the code style guide.
• Audit the FIXME and TODO to figure out what is real versus what can be removed. If it is real, then we should create an issue for it. Be as specific as possible in the write-up.
  • Clean up things as you find them.
• Make GetInfo be uniform. Need a style guide that specifies what the output of this function should look like. [Yuchen]
  • Always use spaces instead of tabs
  • The last character of the returned string should not be a newline.
  • Use the predefined strings in StringUtil
• Make sure that we don't call printf/cout anywhere. Check the source validator script to remove whitelist entries. [Kazuhiro]
  • Change Bw-Tree to not use their own printf. Switch that.
• Audit test case output to reduce the number lines printed. NumericValue::MathTest is the biggest offender. [Haoxiang]
• Finally add stack traces when we throw an exception. [Haoxiang]
• Check whether triggers are still working and whether they can invoke the new UDFs. [Lin]
• Make sure the default size of the LockFreeArray is not 1million. Check uses of this class. Decide whether to get rid of it and replace it with... some other concurrent data structure?
  • Run massif, figure out where we are wasting heap space [Rohit]
  • Replace LockFreeArray with something from libcds [Someone] <-- Andy will write an issue
• Don't create the stats table on first use. Create when the system boots up. [Someone] <-- Andy will write issue
• Rename 'Transaction' to 'TransactionContext' [Siva]
• Go through the issues and remove ones that are fixed, no longer an issue, or stupid. [Andy, Pooja]
  • Put stupid ones in the cookie jar.
• Add Doxygen-style comments to all functions. We should convert existing comments to the new format. [Aaron]
  • Make sure all the comments are in the header files and not the c++ files.
  • Send an email to the mailing list about what style we want to use.
  • Figure out how to automatically enforce this.
  • Add pre-commit hook to make sure that we have the proper doxygen tags for all functions. [Aaron]
• Specify where is each template being used in cuckoo_map.cpp. (https://github.com/cmu-db/peloton/blob/0f3ba1cb4f800580c699111b415d7fb05846e334/src/container/cuckoo_map.cpp#L91) [Tianyi]
• Make the read-write set thread-safe? Can we just use the cuckoo hash map for this? [Pooja]
  • I think we're doing something stupid with copying tuples before hand it off the garbage collector??? Ask Tony to remind us what this was originally about...
• Confirm and remove the unused ClearGarbage function in GCManager to avoid confusion. We should also add more comments/documents to garbage collection if possible. [Lin]
• Change build system to create libpg_query in build folder (instead of thirdparty) (https://github.com/cmu-db/peloton/issues/809#issuecomment-348318817) [Marcel]
• Complete the refactoring of the optimizer's infrastructure and add thorough documentation for the optimizer[Bowei]
• Add comment to explain StatementResult. [Pooja]
• Fix ContainerTuple::GetInfo() to print stringified tuple instead of FIXME. [Prashanth]
• [added on Dec/19] Cleanup TrafficCop (remove some dead code, format, etc.) [Zhixun]
• Fix EXPECT_EQ(false, ...) to EXPECT_FALSE(...) [Pooja]
• Audit the tests which have been commented out and stop the offenders from graduating [Andy]

TODO After Dec 25th

• [added on Dec/19] Add an underscore after each member variable name, e.g. src/include/storage/tile_group.h [Proposed by Tony]
• Make sure that all classes are in the proper namespace. We should add a script to automatically enforce this. #491
  • Fix anything that is not in the 'peloton' namespace [Priyatham]
  • Add 'common' namespace.
  • Add namespaces for sub-directories.
  • Add cheesey check for checking this in the formatter. [Marcel]
  • Make a website that shows pictures of the person that wrote bad code. [Allison]
• Audit the list of directories and see whether to put some directories inside of each other. Here some possible things to move: [Priyatham]
  • 'src/container' -> 'src/common/container'
  • 'src/util' -> 'src/common/util'
  • 'src/threadpool' -> 'src/common/threadpool'
  • 'src/statistics' -> 'src/storage/statistics'
  • Rename 'concurrency' -> 'transaction'
  • Rename 'src/settings' to 'src/setting'
  • Rename 'src/statistics' to 'src/stat'
  • 'src/gc' -> 'src/storage/gc'
  • 'src/trigger' + 'src/expression' + 'src/function' into 'src/type' or 'src/sql' or 'src/query' ????
  • 'src/binder' + 'src/parser' + 'src/optimizer' into 'src/planner'. Current files in 'src/planner' to 'src/planner/plan'
• Remove unused runtime_keys vector from IndexScanDesc here [Pervaze]

Longer Term Things:

• Make all public data members private [Everyone]
• Remove hacky "# of threads + 3" [Rohit]
• Add support for Array [Aaron]
• Audit the number of time that we copy the query result. [Prashasthi]

Things that are harder that we will do later:

• Introduce better Java testing for script/jdbc (maybe JUnit) [Tianyu]
• sqrt, ceil, pow, exp, and many other mathematical functions have LLVM intrinsic implementations that may be faster than calling into C++ [Prashanth]
• Create a script for updating the file header copyright and apply it (can be done with formatter.py -h * once https://github.com/cmu-db/peloton/pull/941 is merged in) [Andy]
• Figure out a way to eliminate singletons (dependency injection vs. manual passing of objects) [Tianyu]