Difference between revisions of "User:Pulkit Mittal/GSOC2014/logs"

From BRL-CAD
(Development Logs)
m (GSOC Period)
Line 56: Line 56:
 
*Added the thread-safe ''getInstancesSafely'' and ''countInstancesSafely''. Also submitted the code in github.
 
*Added the thread-safe ''getInstancesSafely'' and ''countInstancesSafely''. Also submitted the code in github.
  
 +
 +
===Week 2===
 
'''24 May (Sat):'''
 
'''24 May (Sat):'''
 +
* Started working towards making lazy-loading of instances thread safe.
 +
** Created a test which would spawn two threads which try to load instances.
 +
*** In the first half of iterations the threads would try to load same set of instances.
 +
*** In the second half of iterations the threads would try to load different set of instances (not necessary disjoint).
 +
*** For each thread within one iteration an instance would be loaded twice. This checks the case where the instance may or may-not be already loaded and the case where the instance is already loaded.
 +
** The problem that arose during the compilation of the test was that, finally the ''_mainRegistry'' was being used, and I had very little idea about the schemas and registries. It could not find the ''schema.h'' file.
 +
* Couldn't work the earlier half of the day as I was traveling.
 +
 +
'''25 May (Sun):'''
 +
* Got the thread safety test for lazy-loading to compile by doing certain changes in the ''src/cllazyfile/CMakeLists.txt''.
 +
** This was done by including the directory ''${CMAKE_BINARY_DIR}/schemas/sdai_cd209'' for the above test.
 +
** The restriction that the above changes imposed on ''lazy_thread_safety_test'' was that, now it has to be given the the step file ''data/cd209/ATS1-out.stp'' as input.
 +
* A potential bug was found while running the above test. The bug would appear if we follow the following steps
 +
*# ''lazyInstMgr * mgr = new lazyInstMgr;''
 +
*# ''mgr->initRegistry( SchemaInit );''
 +
*# ''mgr->openFile( fileName );''
 +
*# ''delete mgr''
 +
*# ''lazyInstMgr * mgr = new lazyInstMgr;''
 +
*# ''mgr->initRegistry( SchemaInit );''
 +
** The last step would cause assertion failure in function ''setRegistery''. (i.e. ''assert( _mainRegistry == 0 )''). This was solved by explicitly initializing ''_mainRegistry'' with '0' inside the ''lazyInstMgr'' constructor.
 +
* When the test was run on the original ''loadInstance'' function, it gave assertion error related to the input stream.
 +
* Saw the comments posted by Mark on my earlier commits. I decided to make corresponding changes in the code, once I had made lazy-loading thread safe
 +
 +
'''26 May (Mon):'''
 +
* A thread-safe version of ''loadInstance'' was created. For this a fat lock was used. It is possible that we can make the lock application finer, however that has been left for later. This version passed the test successfully.
 +
** However their appeared to be some problem in the step file, because loading of one particular instance always failed, irrespective of the number of threads. A temporary fix was applied by removing a newline character from the step file.
 +
** In the ''loadInstance'' function, their was an issue of a warning ''instance #... not found in any section'' appearing even if the instance was found in the ''_instancesLoaded'' list. The fix was trivial
 +
* A thread safe counterpart to ''mgr->getAdapter()->FindFileId( instanceID )->GetSTEPentity()'' was made. This was slightly complicated then the previous features as it consisted of 2 complex steps in different classes. Both being thread unsafe.
 +
** For this each thread was assigned its own mgrNodeHelper. This mgrNodeHelper was created once, for each thread and reused on later invocations.
 +
** Dependency was taken on thread safe ''loadInstanceSafely''
 +
** The counterpart was also successfully tested
 +
 +
'''27 May (Tue):'''
 +
* As per the advice given by Mark, the macro ''HAVE_STD_THREAD'' was utilized in ''CMakeLists.txt'', including header files, mutex / thread safe functions declarations etc.
 +
** This opportunity was also used to clean up the ''CMakeLists.txt''
 +
* Fixed the newline issue encountered on 26th May. This was done by modifying the lazyfile parser by instructing it to ignore the newlines in such cases. The temporary fix was reverted back as it was no longer required.
 +
* Prepared the changes done from 24th May in the form of multiple commits. Also the thread safe code was clubbed together wherever possible for easy identification.

Revision as of 14:58, 27 May 2014

Development Logs

Project Details

Project Name STEP Libraries: Improving Thread Safety and Performance
Project Student Pulkit Mittal
IRC(nick) hoiji
Github (handle) hoiji09

Milestones

Compulsary

  • Thread Safety in cllazyfile Status: Doing
  • Thread Safety in clstepcore Status: Not Started
  • Thread Safety in cleditor Status: Not Started
  • Thread Safety in cldai Status: Not Started
  • Introduce multithreading cllazyfile Status: Not Started

Bonus

In this section I will list down all the single threaded performance optimizations which are done in the GSOC period.


GSOC Period

Week 1

17 May (Sat):

  • Figured out various functionalities in cllazyfile step library which should be made thread safe.
    These are
    1. Opening multiple files in parallel.
    2. (lazy) loading separate SDAI_Application_instance in parallel.
    3. Reading the forward / backward references of instances in parallel.
    4. Finding instances belonging to same / different types in parallel.
    5. Initializing / Setting the registry in parallel.
    6. Registering Data Sections in parallel
  • Strategizing how the various features will be added into the code. Firstly a test will be created to note the existance of thread safety for the above features. If the test fail then appropriate coding will be done

18 May (Sun):

  • A test was created in lazy_thread_safety_test.cc file, to check the concurrent read safety of getFwdRefs() and getRevRefs() and expectedly it failed.
  • Note: to run the test, the CMakeFiles: cmake/SC_CXX_schema_macros.cmake & src/cllazyfile/CMakeLists.txt were slightly messed up.

19 May (Mon):

  • Introduced getFwdRefsSafely and getRevRefsSafely which are thread safe. The thread safety test in lazy_thread_safety_test.cc passed. Submitted code through git in a branch called hj/cllazyfile-thread-safety.
  • Discussed with mentor about the importance order of functionalities mentioned above. Hence will be covering the functionalities in the following order 3(done)>4>2>1>6

20-21 May (Tue, Wed):

  • Noticed that getFwdRefsSafely and getRevRefsSafely are consume memory each time they are called (as separate clones are made for each invocation). (Blame the use of Judy Structure) The memory is freed only when destructor of lazyInstMgr is called. One way around this problem was to let the user call a function (say) returnFwdRefsSafely which would release the space related to a clone. Hence, I spent 2 days trying to modify the original judy code so that it allows closure of clones. But as I was not able to get a breakthrough, I have left this task for later.

22 May (Thu):

  • Created a test to check the thread safety of getInstances (i.e. finding instances belonging a specific type). As expected the test failed when the number of invocations were very high. This is because getInstances internally uses find operation which is not thread safe.

23 May (Fri):

  • Added the thread-safe getInstancesSafely and countInstancesSafely. Also submitted the code in github.


Week 2

24 May (Sat):

  • Started working towards making lazy-loading of instances thread safe.
    • Created a test which would spawn two threads which try to load instances.
      • In the first half of iterations the threads would try to load same set of instances.
      • In the second half of iterations the threads would try to load different set of instances (not necessary disjoint).
      • For each thread within one iteration an instance would be loaded twice. This checks the case where the instance may or may-not be already loaded and the case where the instance is already loaded.
    • The problem that arose during the compilation of the test was that, finally the _mainRegistry was being used, and I had very little idea about the schemas and registries. It could not find the schema.h file.
  • Couldn't work the earlier half of the day as I was traveling.

25 May (Sun):

  • Got the thread safety test for lazy-loading to compile by doing certain changes in the src/cllazyfile/CMakeLists.txt.
    • This was done by including the directory ${CMAKE_BINARY_DIR}/schemas/sdai_cd209 for the above test.
    • The restriction that the above changes imposed on lazy_thread_safety_test was that, now it has to be given the the step file data/cd209/ATS1-out.stp as input.
  • A potential bug was found while running the above test. The bug would appear if we follow the following steps
    1. lazyInstMgr * mgr = new lazyInstMgr;
    2. mgr->initRegistry( SchemaInit );
    3. mgr->openFile( fileName );
    4. delete mgr
    5. lazyInstMgr * mgr = new lazyInstMgr;
    6. mgr->initRegistry( SchemaInit );
    • The last step would cause assertion failure in function setRegistery. (i.e. assert( _mainRegistry == 0 )). This was solved by explicitly initializing _mainRegistry with '0' inside the lazyInstMgr constructor.
  • When the test was run on the original loadInstance function, it gave assertion error related to the input stream.
  • Saw the comments posted by Mark on my earlier commits. I decided to make corresponding changes in the code, once I had made lazy-loading thread safe

26 May (Mon):

  • A thread-safe version of loadInstance was created. For this a fat lock was used. It is possible that we can make the lock application finer, however that has been left for later. This version passed the test successfully.
    • However their appeared to be some problem in the step file, because loading of one particular instance always failed, irrespective of the number of threads. A temporary fix was applied by removing a newline character from the step file.
    • In the loadInstance function, their was an issue of a warning instance #... not found in any section appearing even if the instance was found in the _instancesLoaded list. The fix was trivial
  • A thread safe counterpart to mgr->getAdapter()->FindFileId( instanceID )->GetSTEPentity() was made. This was slightly complicated then the previous features as it consisted of 2 complex steps in different classes. Both being thread unsafe.
    • For this each thread was assigned its own mgrNodeHelper. This mgrNodeHelper was created once, for each thread and reused on later invocations.
    • Dependency was taken on thread safe loadInstanceSafely
    • The counterpart was also successfully tested

27 May (Tue):

  • As per the advice given by Mark, the macro HAVE_STD_THREAD was utilized in CMakeLists.txt, including header files, mutex / thread safe functions declarations etc.
    • This opportunity was also used to clean up the CMakeLists.txt
  • Fixed the newline issue encountered on 26th May. This was done by modifying the lazyfile parser by instructing it to ignore the newlines in such cases. The temporary fix was reverted back as it was no longer required.
  • Prepared the changes done from 24th May in the form of multiple commits. Also the thread safe code was clubbed together wherever possible for easy identification.