Author Topic: m_TopNameSpaces in TokensTree class seems useless  (Read 5024 times)

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5229
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
m_TopNameSpaces in TokensTree class seems useless
« on: October 21, 2010, 07:36:08 am »
@all

When reviewing the code of token class, I found that: m_TopNameSpaces is not needed any more.

If you search the member variable "m_TopNameSpaces" usage by ThreadSearch, you will notice that no one use it.

So, shall we just delete this un-used variable?

It is declared in: token.h line 285.


If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5229
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: m_TopNameSpaces in TokensTree class seems useless
« Reply #1 on: October 21, 2010, 08:14:05 am »
Also, this variable

Code: [Select]
m_Modified
is only used when we need to cache the tokens to hard-disk.

Code: [Select]
bool Parser::CacheNeedsUpdate()
{
    if (m_UsingCache)
    {
        wxCriticalSectionLocker locker(s_TokensTreeCritical);
        return m_TokensTree->m_Modified;
    }
    return true;
}

But, In-fact we don't use cache at all, so should be removed too.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9580
Re: m_TopNameSpaces in TokensTree class seems useless
« Reply #2 on: October 21, 2010, 08:27:50 am »
But, In-fact we don't use cache at all, so should be removed too.
After a short discussion about if we need the cache at all anymore I'd say if this turns out to be of no use anymore we should rather remove the whole cache facility, not only this variable. Otherwise caching is broken.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: http://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: http://wiki.codeblocks.org/index.php?title=FAQ

Offline ollydbg

  • Developer
  • Lives here!
  • *****
  • Posts: 5229
  • OpenCV and Robotics
    • Chinese OpenCV forum moderator
Re: m_TopNameSpaces in TokensTree class seems useless
« Reply #3 on: October 21, 2010, 10:02:46 am »
agreed.

The cache was too big(eg. the codeblocks.cbp should have 100M memory size) to save to hard disk.

So, I think we don't need cache persistent facility anymore.
If some piece of memory should be reused, turn them to variables (or const variables).
If some piece of operations should be reused, turn them to functions.
If they happened together, then turn them to classes.

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9580
Re: m_TopNameSpaces in TokensTree class seems useless
« Reply #4 on: October 21, 2010, 12:48:38 pm »
The cache was too big(eg. the codeblocks.cbp should have 100M memory size) to save to hard disk.
The case wasn't saved to the project file abut another (binary based) file. So the size would not be an issue. The question is if we need this to improve speed of scanning through large projects...
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: http://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: http://wiki.codeblocks.org/index.php?title=FAQ

Offline ironhead

  • Almost regular
  • **
  • Posts: 210
Re: m_TopNameSpaces in TokensTree class seems useless
« Reply #5 on: October 21, 2010, 01:08:46 pm »
The cache was too big(eg. the codeblocks.cbp should have 100M memory size) to save to hard disk.
The case wasn't saved to the project file abut another (binary based) file. So the size would not be an issue. The question is if we need this to improve speed of scanning through large projects...

To that end, wouldn't it make sense to use the cache on project startup (i.e. load the file cache into memory) versus parsing all the project files again?  Perhaps this would improve the initial project parsing time?

Offline Loaden

  • Lives here!
  • ****
  • Posts: 1014
Re: m_TopNameSpaces in TokensTree class seems useless
« Reply #6 on: October 21, 2010, 01:21:45 pm »
The cache was too big(eg. the codeblocks.cbp should have 100M memory size) to save to hard disk.
The question is if we need this to improve speed of scanning through large projects...
But we need more IO(read/write) time, when save to hard disk, we need loop the whole tree, their need more time.
So, I personally think this is not needed.

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9580
Re: m_TopNameSpaces in TokensTree class seems useless
« Reply #7 on: October 21, 2010, 08:21:12 pm »
So, I personally think this is not needed.
I personally believe it would only make sense for system headers, like STL, BOOST or alike. Or simply the compiler's include path would be something where a cache might e useful.

Besides that I see no use, indeed.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: http://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: http://wiki.codeblocks.org/index.php?title=FAQ