Author Topic: autorevision.h  (Read 7440 times)

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5529
autorevision.h
« on: August 01, 2007, 07:09:14 pm »
@Thomas :
is it possible that the header file becomes fixed, and just exports the types of the variables, and the actual values are generated in a corresponding cpp file ?
Now when something changes, 4 files have to recompile, because they include the changed header, resulting also in 2 link steps (codeblocks dll and app).

This way only one file should recompile, and 1 link step is needed (probably the dll, so the cpp file should be added to the dll target of the cbp files (win/linux) and the makefile).

In case you agree, could you arrange this ?

Offline thomas

  • Administrator
  • Lives here!
  • *****
  • Posts: 3979
Re: autorevision.h
« Reply #1 on: August 02, 2007, 11:03:06 am »
It is probably possible, but I'm not going to try it. You're of course free to try, but I recommend against it.
The possible gains are small and the possible problems are plenty, and as it is now, it works.

The fact that 4 files are recompiled is just sloppy programming. It is only really necessary to recompile one file (configmanager-revision.cpp). If the application, the splash screen, and the about dialog used the static wrapper functions, they wouldn't have to be rebuilt at all.
However, the combined compile time for those 3 files is well under a second, while relinking the SDK takes some 10-15 seconds, so it's quite irrelevant. Don't optimise where there is no benefit to be expected.
Relinking the SDK is what is really painful, but it has to be done either way (as the object file could only be linked with the SDK). This means that the application will be relinked too, either way.

From an organisational point of view, where to put the .cpp file? There would have to be an extra commandline option to specify a separate path, or it would go into include.
How to compile the .cpp file? It can't be added to the project. As the file is not under revision control, it does not originally exist. The IDE would notice that while loading the project.
Insert a dummy file? Modify the project while it is building using a script? Any other ideas? :)
All this to gain half a second of compile time?
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5529
Re: autorevision.h
« Reply #2 on: August 02, 2007, 03:51:22 pm »
4 files recompile, and relink --> takes much more as the seconds you describe ;-)
Relinking (sdk or app) sometimes takes nearly 1 minute !!

But we can already limit towards only using the configmanager-revision.cpp provided wrappers. I will have a look at it.

Relinking SDk --> new dll, but for that the import library shouldn't change, so the app should not relink then.

Offline Biplab

  • Developer
  • Lives here!
  • *****
  • Posts: 1874
    • Biplab's Blog
Re: autorevision.h
« Reply #3 on: August 02, 2007, 04:49:44 pm »
4 files recompile, and relink --> takes much more as the seconds you describe ;-)
Relinking (sdk or app) sometimes takes nearly 1 minute !!

It takes more time. My PC in Lab (P4-2.4 GHz, 2GB RAM) takes approximately 1-2 minutes (It varies depending upon the number of other apps running)!! :)
Be a part of the solution, not a part of the problem.

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5529
Re: autorevision.h
« Reply #4 on: August 02, 2007, 11:01:10 pm »
Thomas, although you don't seem to agree, you put the basis for the same thing. Only the requested cpp file by me, already exists : configmanager-revision.cpp. So in the end we do agree.

I have adjust the other client of autorevision.h, so they use the static's from the ConfigManager.

This ends up in these changes :
dlgabout.cpp:
 const wxString revision(wxT(SVN_REVISION)); --> ConfigManager::GetRevisionString()

splashscreen.cpp:
 const wxString revision(wxT(SVN_REVISION)); --> ConfigManager::GetRevisionString()

appglobals.cpp:
   SVN_REVISION --> ConfigManager::GetRevisionString()

Now get rid of the f*** defines :
configmanager-revision.cpp:
wxString ConfigManager::GetRevisionString()
{
    return wxString(wxT(SVN_REVISION));
}

becomes :
wxString ConfigManager::GetRevisionString()
{
    return autorevision::svnRevision;
}

--> SVN_REVISION no longer needed --> don't put it during the generation

   **** startherepage show the svn date <<---> about : build date --> could be confusing

wxString ConfigManager::GetSvnDate()
{
    return wxString(wxT(SVN_DATE));
}

should become :

wxString ConfigManager::GetSvnDate()
{
    return autorevision::svnDate;
}

as such we should adjust autorevision.cpp:
    if(do_std)
        fprintf(header, "\tconst std::string svn_date_s(%s);\n", date.c_str());
    if(do_wx)
        fprintf(header, "\tconst wxString svnDate(%s);\n", date.c_str());
 and remove the define SVN_DATE
additional change needed : codeblocks.cbp file --> in the call to autorevision add the command line arg +wx


So now also the windows cbp file generates as wxString (like the autotools and the linux cbp), consistency is sometimes not our best thing ;-)

I also got completely rid of the stupid defines, all is now available as statics const's. The new generated header autorevison.h will show these changes, and has a note at the top stating it should not be included by anyone else then configmanager-revision.cpp.


Offline thomas

  • Administrator
  • Lives here!
  • *****
  • Posts: 3979
Re: autorevision.h
« Reply #5 on: August 03, 2007, 09:51:51 am »
Quote
Thomas, although you don't seem to agree, you put the basis for the same thing. Only the requested cpp file by me, already exists : configmanager-revision.cpp.
It's not that I don't agree, I just think the gains are worth the pain and the possible troubles. But like I said, you're free to do it. As long as it doesn't break anything, I'm fine :)

Quote
Now get rid of the f*** defines :
There was a reason for the presence of those #defines, and the wrapper functions did not use them inadvertently either, but well... never mind. They're gone now, and it seems to work by accident, so leave it :)
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5529
Re: autorevision.h
« Reply #6 on: August 03, 2007, 10:12:37 am »
I checked the uses of the defines, and you can see we can do very well without them. Although when I read your words, you seem to think they were needed, could you explain where they should have been used then. You can get everything out of the namespace const's, so why 2 of them also as define ??

Offline thomas

  • Administrator
  • Lives here!
  • *****
  • Posts: 3979
Re: autorevision.h
« Reply #7 on: August 03, 2007, 11:03:14 am »
There are conditions in which global const wxString objects don't work reliably between two shared modules. I don't recall what exactly the conditions were, but using the define to create a "local" copy per shared module always works reliably.
"We should forget about small efficiencies, say about 97% of the time: Premature quotation is the root of public humiliation."