Editing User:Pulkit Mittal/GSOC2014/logs

From BRL-CAD

Warning: You are not logged in. Your IP address will be publicly visible if you make any edits. If you log in or create an account, your edits will be attributed to your username, along with other benefits.

The edit can be undone. Please check the comparison below to verify that this is what you want to do, and then save the changes below to finish undoing the edit.
Latest revision Your text
Line 16: Line 16:
 
===Compulsary===
 
===Compulsary===
 
*Thread Safety in cllazyfile ''Status'': Review Awaiting  
 
*Thread Safety in cllazyfile ''Status'': Review Awaiting  
*Thread Safety in clstepcore ''Status'': Review Awaiting
+
*Thread Safety in clstepcore ''Status'': Started
*Thread Safety in cleditor ''Status'': Review Awaiting
+
*Thread Safety in cleditor ''Status'': Not Started
*Thread Safety in cldai ''Status'': Review Awaiting
+
*Thread Safety in cldai ''Status'': Not Started
 +
*Introduce multi-threading cllazyfile ''Status'': Not Started
  
 
===Bonus===
 
===Bonus===
In this section I will list down all other modifications which I did during the GSOC period.
+
In this section I will list down all the single threaded performance optimizations which are done in the GSOC period.
* Single threaded performance optimizations
+
 
** Removed ''istream *in2'' from ''STEPfile::AppendFile''. Originally for the second pass a new ''istream (in2)'' was being created and the ''.step'' file was being opened again. This was prevented by directing the original ''istream (in)'' to the beggining of the ''.step'' file.
 
* Code Refractoring
 
** In libraries clstepcore and cldai, there were few classes which were very similar. The logic common to these classes was indentified and removed, a new class was created which had that common logic. The original classes were made to inherit from the new class. This removed duplicated code and also made making changes (for thread safety) easier.
 
  
  
Line 379: Line 377:
 
* Had to devote most of the whole day to checking major scripts (a part of my teaching assistant duties. Thankfully now I am free from that)
 
* Had to devote most of the whole day to checking major scripts (a part of my teaching assistant duties. Thankfully now I am free from that)
  
 
 
=== Week 9 ===
 
 
'''12 July (Sat)'''
 
'''12 July (Sat)'''
 
* Modified ''add_schema_dependent_test'' module.  
 
* Modified ''add_schema_dependent_test'' module.  
 
** The assertion failures before was due to the test file not being equipped with the ''HAVE_STD_THREAD'' macro.
 
** The assertion failures before was due to the test file not being equipped with the ''HAVE_STD_THREAD'' macro.
 
** The CMakeFile associated with ''add_schema_dependent_test'' was changed and all the tests except of 'test_inverse_attr3' are passing.
 
** The CMakeFile associated with ''add_schema_dependent_test'' was changed and all the tests except of 'test_inverse_attr3' are passing.
 
'''13 July (Sun)'''
 
* Made ''STEPattributeList'' thread safe.
 
** The functions ''STEPattributeList::[]'' & ''STEPattributeList::push'' were identified as vulnerable.
 
** Instead of declaring a new mutex to protect the state, the mutex defined in its superclass ''SingleLinkList'' was used.
 
* Redefined ''GetMiEntity'' API belonging to ''SDAI_Application_instance'' class.
 
** The function now accepts ''const char *'' (instead of ''char *'') as parameter.
 
 
'''14 July (Mon)'''
 
* Removed the infinite recursion bug in ''STEPattribute''.
 
** This bug was identified few days ago when I was going through the code. Finally I was able to discuss it with the mentor.
 
** The bug would surface if a ''STEPattribute'' instance which has been redefined, has its ''ShallowCopy'' function invoked.
 
 
'''15 July (Tue)'''
 
* Added test for checking thread safety of ''SDAI_Application_instance'' class.
 
** This test would check thread safety for ''AppendMultInstance'' & ''GetMiEntity'' functions in class ''SDAI_Application_instance'' by using two threads two invoke ''AppendMultInstance'' on the same ''SDAI_Application_instance'' object. A third thread would call ''GetMiEntity'' simultaneously.
 
** Rest of the functions were not checked since the locking in them was trivial.
 
** The test failed on the original code.
 
 
'''16 July (Wed)'''
 
* Made ''SDAI_Application_instance'' thread safe.
 
** This was done by introducing a mutex for each ''SDAI_Application_instance''.
 
** ''AppendMultInstance'' was made thread safe by doing hand-over-hand locking.
 
** GetMiEntity was made thread safe acquiring locks in increasing order and releasing locks in decreasing order.
 
** The checks in sdaiApplication_instance_thread_safety_test now pass.
 
 
'''17 July (Thu)'''
 
* Travelling home.
 
 
'''18 July (Fri)'''
 
* Work done from 13 July was divided into different commits and pushed into github.
 
 
 
 
=== Week 10 ===
 
'''19 July (Sat)'''
 
* Locking was done in ''ExpDict.cc''.
 
** There were some functions in ''ExpDict.cc'' that were not properly locked before.
 
 
'''20 July (Sun)'''
 
* Made ''sdaiSelect::CanBe( BASE_TYPE )'' thread safe.
 
* Started strategizing how to make ''STEPcomplex'' thread safe.
 
 
'''21 July (Mon)'''
 
* Travelling back to university.
 
 
'''22 July (Tue)'''
 
* Work done from 19 July was organized into different commits and pushed into github.
 
* Fixed locking in ''sdaiApplication_instance.cc''. The ''STEPattributeList'' data was being iterated and modiefied by the ''SDAI_Application_instance'' class without invoking the mutex of the ''STEPattribute'' class. This was fixed in this commit.
 
* Minor ''STEPcomplex'' optimization.
 
** Collapsed some if conditions into a while condition without changing the semantics
 
 
'''23 July (Wed)'''
 
* Made ''STEPcomplex'' thread safe.
 
** The assumption used was that ''STEPcomplex'' class wont in future provide an API through which threads can remove / insert a ''STEPcomplex'' node.
 
** Three already declared mutexes were used in for this purpose:
 
**# A recursive mutex for each ''STEPcomplex'' node (borrowed from the superclass ''SDAI_Application_instance'')
 
**# A recursive mutex belonging to ''AttrDescriptorList'' class (originally defined in ''SingleLinkList'' class)
 
**# A recursive shared mutex belonging to ''EntNode'' class.
 
*** The fixed order between the locks was: (2) > (1). (3) was independent.
 
 
 
'''24 July (Thu)'''
 
* Work done from 22 July was organized into different commits and pushed into github.
 
* Thread safety in ''EntList''.   
 
** Under the assumptions that an ''EntList'' object cannot be inserted in between two ''EntList'' objects and cannot be removed from a list of ''EntList'' objects, making this class thread safe became trivially easy.
 
** One mutex per ''EntList'' object was introduced to support appending of ''EntList'' objects. This feature would be used by its subclass (''MultList'')
 
 
 
'''25 July (Fri)'''
 
* Thread Safety in ''SimpleList''   
 
** Used ''sharedMtxP'' defined in ''EntNode'' class to protect ''EntNode'' list structure which was being accessed in functions of ''SimpleList'' class
 
* Thread safety in ''MultList''   
 
** The mutex defined in the superclass ''EntList'' was used in the function appendList.
 
 
 
 
=== Week 11 ===
 
'''26 July (Sat)'''
 
* Thread safety in ''JoinList'', ''AndList'' & ''AndOrList''
 
** ''sharedMtxP'' of ''EntNode'' class was invoked to keep the list of ''EntNode'' consistent whenever the ''EntNode'' list was being used multiple times in a function belonging to any of the above classes.
 
 
'''27 July (Sun)'''
 
* Thread safety in ''OrList''   
 
** Strategy adopted was similar to ''AndList''. However the mutex belonging to the superclass ''EntList'' was used to protect various counters (choiceCount, choice) used by ''OrList''
 
 
'''28 July (Mon)'''
 
* Thread safety in ''ComplexList''
 
** Like the classes before, used ''sharedMtxP'' to protect the ''EntNode'' list whenver it was being iterated upon.
 
** Also added a public mutex to protect the public next pointer. Resposibilty of using it will fall to the class which is using next pointer (''ComplexCollect'')
 
 
'''29-31 July, 1 August (Tue-Fri)'''
 
* Was down with fever (the first couple of days), followed by eyes pain and weakness. Was advised to take a bed rest.
 
 
 
 
=== Week 12 ===
 
'''2 August (Sat)'''
 
* Thread Safety in ''ComplexCollect''   
 
** A mutex was introduced to proctect the counter and the pointer to ''clists''.
 
** The insert and remove functions were tricky to make thread safe as ''clists'' may or may-not have been initialized. Hand-over-hand locking was used.
 
** Note: In the function ''ComplexCollect::supports( EntNode )'', there was a piece of code present in which the next and prev pointer of an EntList were being modified.
 
*** This could have ruined the thread safe strategy for making ''EntList'' thread safe. (violated our assumption)
 
*** The reason it didn't was that, that ''EntList'' was created locally.
 
 
'''3 August (Sun)'''
 
* Work done from 24 July was organized into commits and pushed to github.
 
* Made ''ReplicateList'' in ''cleditor'' thread safe.
 
** Used ''mtxP'' of the superclass ''SingleLinkList'' to ensure thread safety
 
* Thread safety in ''CmdMgr'' of ''cleditor''
 
 
'''4 August (Mon)'''
 
* Not much work done as it was a heavy day in college
 
 
'''5 August (Tue)'''
 
* Made ''DirObj'' of ''clutils'' thread safe.
 
** This was achieved introducing a mutex in the ''DirObj'' class.
 
** Course-Level (fat) Locking was done only in publically accessed function.
 
 
'''6 August (Wed)'''
 
* Work done from 3 August was organized into commits and pushed to github.
 
 
'''7 August (Thu)'''
 
* Converted ''GetKeyword'' to ''FillKeyword''.
 
** In the original function a ''static string'' was being used.
 
** The dependency on the static string was removed by passing a string as a parameter from the caller to callee, so that the function could operate without worrying about thread safety.
 
** The function name change mimics the change in the semantics of the function.
 
** The various callers were modified to incorperate this change
 
 
'''8 August (Fri)'''
 
* Optimized ''ReadReal''. Replaced calls to input stream peek & get with judicious use of get.
 
* Work done from 7 August was organized into commits and pushed to github.
 
 
 
 
=== Week 13 ===
 
'''9 August (Sat)'''
 
* No changes were done to ''Sdai*'' files of the ''cleditor'' library as those files were generated by ''exp2cxx''.
 
** Any modifications would be lost if ''exp2cxx'' was used to regenerate them.
 
* Went through ''STEPfile'' class to determine the areas of thread safety violations.
 
 
'''10 August (Sun)'''
 
* Inspired by a comment written by a developer, removed ''istream *in2'' from ''STEPfile::AppendFile''.
 
** Originally for the second pass a new ''istream (in2)'' was being created and the ''.step'' file was being opened again.
 
** The above was prevented by directing the original istream (in) to the beggining of the ''.step'' file.
 
** The work was commited and pushed into github.
 
 
'''11 August (Mon)'''
 
* Protected ''_headerId'' in ''STEPfile'' ''(cleditor)''.
 
** ''_headerId'' is a counter used in ''STEPfile'' class.
 
** Its value could change in ''ReadHeader'' and ''ReadExchangeFile'' methods.
 
** Hence a mutex was introduced specially to protect it.
 
* Added ''GetApplication_instanceFromFileId'' method in ''InstMgr'' class.
 
** The ''SDAI_Application_instance * GetApplication_instance( MgrNode * )'' and'' MgrNode * FindFileId( int )'' methods of class ''InstMgr'' were often being used together in the class ''STEPfile''.
 
** Therefore a special function was created in class ''InstMgr'' of the form ''SDAI_Application_instance * GetApplication_instanceFromFileId( int )'' which would take care of locking and consistency issues.
 
** This function was then used in ''STEPfile''.
 
 
'''12 August (Tue)'''
 
* Safe ''InstMgr'' iteration in ''STEPfile''.
 
** Variables of class ''InstMgr'' were being iterated upon in class ''STEPfile'' through the variables ''_instances'' (including its get function ''instances()'') and ''_headerInstances''.
 
** The mtx defined in ''InstMgr'' was invoked to make them safe.
 
 
'''13 August (Wed)'''
 
* Safe ''attributes'' iteration in ''STEPfile''.
 
** This was done by using mutexes defined for ''SDAI_Application_instance'' & ''SingleLinkList'' classes.
 
* It was decided that as this class acts like a parser, further locking in this class would be dependent upon how it will be used in a multithreaded way.
 
** The current structure of the class was restricting any potential multithreaded use.
 
** Example: The variable like ''_fileName'' restricts this class to parse only one file at a time. If we want to parse more then one then this would have to change.
 
* Thread safety in ''SDAI_Application_instance__set''
 
** This was done by introducing a ''recursive_mutex'' in the ''SDAI_Application_instance__set'' class.
 
* Work done from 11 August was organized into commits and pushed to github.
 
 
'''14 Augest (Thu)'''
 
* Removed compile time warning from the previous day commit.
 
* Added class ''SDAI__set'' in ''cldai'' library
 
** It was noticed that some classes in this library which were very similar: ''SDAI_DAObject__set'', ''SDAI_Entity_extent__set'' & ''SDAI_Model_contents__list''
 
** The logic common to these classes was indentified and removed, a new class (''SDAI__set'') was created which had that common logic.
 
** The original classes were made to inherit from the new class.
 
** This removed duplicated code and also made making changes (for thread safety) easier.
 
* Made ''SDAI__set'' thread safe
 
** This was done by introducing a mutex in the ''SDAI__set'' class.
 
 
'''15 Augest (Fri)'''
 
* Added ''SDAI__set::Remove( SDAI_ptr )'' function
 
** The semantics of this function would be similar to ''SDAI__set::Remove( SDAI__set::Index( SDAI_ptr ) )''. The only difference would be that in the new function both the operations are done under a single lock.
 
** Wherever the old ''Remove( Index( SDAI_ptr ) )'' code snippet was found, it was replaced by the new ''Remove( SDAI_ptr )'' function.
 
* One more subclass of ''SDAI__set''
 
** Adding the ''Remove( SDAI_ptr )'' API to ''SDAI__set'' made ''SDAI_Application_instance__set'' a candidate for inheriting common logic from ''SDAI__set''.
 
** Note: This class was already made thread safe in one of the previous commits. This commit only removes the now apparent duplicate code.
 
* Work done from 14 August was organized into commits and pushed to github.
 
 
 
 
=== Week 14 ===
 
'''16 Augest (Sat)'''
 
* On the advice of ''starseeker'' I decided to perform the ''acid test'' by running my version of stepcode through ''step-g'' converter.
 
** Set up ''brlcad''.
 
** Ran ''step-g'' converter on a sample ap203 file.
 
 
'''17 Augest (Sun)'''
 
* Tried to run ''step-g'' using my stepcode version. Unfortunately I was not successful partially due to the following reasons.
 
** To start with, a lot of time was wasted, identifying the CMake file in brlcad in which I should set ''CMAKE_CXX_FLAGS'' (or equivalent) with ''-std=c++0x'' so as to run my version of stepcode which uses ''std::mutex'' & ''std::thread''
 
** Secondly apart from the changes I did in stepcode through ''hj/thread-safety'' branch the stepcode version in ''brlcad/src/other/stepcode'' had many differences.
 
*** It initially gave many build errors.
 
*** When it finally ran it gave a seg-fault on the piece of code I have never changed.
 
* Added ''NO_REGISTRY'' macro to ''lazy_thread_safety_test''
 
** ''lazy_thread_safety_test'' had 2 checks which were schema dependent. These checks are now wrapped in ''NO_REGISTRY'' macro.
 
 
'''18 Augest (Mon)'''
 
* Utilized ''sc_mutex'' in ''cllazyfile''
 
** Earlier ''std::mutex'' was being used.
 
** It was replaced with ''sc_mutex'' which acts like a wrapper to ''std::mutex''.
 
* Added ''sc_thread.h'' in ''clutils''
 
** The class acts like a wrapper to some of the thread functionalities (like ''get_id()'') which is being used in stepcode.
 
** If the compiler does not supports thread functionalities then this class provides dummy types and functionalities.
 
** ''instMgrAdapter'' & ''lazyTypes.h'' were modified so as to take dependency on ''sc_thread''.
 
* As a result of above two changes ''HAVE_STD_THREAD'' macros were removed from cllazyfile library *.cc and *.h as classes ''sc_thread'' and ''sc_mutex'' internally handles the macros.
 
* Added exit(success/fail) in thread-safety test
 
* Organized the work done from 17 augest into commits and pushed into github
 
* Merged remote-tracking branch ''origin/master'' into ''hj/thread-safety''
 
** As I had never merged the master branch into the branch I was working upon ever since I started working on this project, I was 109 commits ahead and 23 commits behind the master branch.
 
** I merged those the 23 commits into my ''hj/thread-safety'' branch. The only conflict was in ''src/clstepcore/instMgr.cc'' which was handled by me.
 

Please note that all contributions to BRL-CAD may be edited, altered, or removed by other contributors. If you do not want your writing to be edited mercilessly, then do not submit it here.
You are also promising us that you wrote this yourself, or copied it from a public domain or similar free resource (see BRL-CAD:Copyrights for details). Do not submit copyrighted work without permission!

To edit this page, please answer the question that appears below (more info):

Cancel Editing help (opens in new window)