Hi,

In the following bug report, I attached a minor patch for the pascal scanner in fcl-passrc.
http://bugs.freepascal.org/view.php?id=17237

I introduced two new tokens. Without this it is not possible to rebuild a source code unit from
a token list.

I cannot accept this patch as it is.

It breaks the parser, more specifically NextToken. The parser ignores whitespace. It should also ignore the new tokens.

Other than that, I doubt the usefulness of this patch. You are on the wrong track with what you attempt to do.

Michael

In our previous episode, Michael Van Canneyt said:

He wants the source as a kind of DOM model. That is IMHO a valid use, but I doubt it is unifiable with the current parser.

This is typically one of the reasons why the "one parser to rule them all" approach is flawed.

Oh, I'll take another look. Let me explain what I'm doing - maybe it makes more sense then. I'm using the tokenizer/scanner and generating a list of all tokens. Then search for specific areas in the code using the tokens (uses clauses, begin..end pares of procedures or methods), then insert timing code, generate the units with newly included code, do a compile so I can get profiling/timing information. So far this works just fine for me, and already found some "hot spots" in my DocView code.

After all this, I Googled profilers to see what everything they check for. I searching, I stumbled accross the fpprofiler which is exactly what I was trying to do (don't you just hate that - after you wrote some code). Funny enough, the fpprofile also only uses the pascal scanner.

I tried using the parser as you suggested in a private email yesterday, but that made my profiler 2x slower at processing my source code (I used EpikTimer to get high precision timing results). The scanner seems good enough for what I want. But without differentiating between general whitespace and EOL (required) and Tab (optional), I can't regenerate my source code units.

Anyway, I abandoned my profiling code and started working with fpprofiler (it was a bit further along than my own attempt), but it wasn't touched for over 2 years, so didn't compile. It was also riddled with memory leaks (sorry Darius Blaszyk). I fixed a lot of things including memory leaks, removed the custom parser to rather use the one included in FCL etc.. But the final step is tokenizing the EOL character correctly - which fcl-passrc doesn't do.

BTW:
fcl-passrc can't parse program units either, but I'm already working on a patch for that.

Other than that, I doubt the usefulness of this patch. You are on the wrong track with what you attempt to do.

Based on the information above, and what I'm trying to do, do you still think I'm on the wrong track? Otherwise, what do you suggest? I think all I need to do, is add the LineEnding token to the parser to make sure I didn't break it (sorry, I never tested the parser before I generated the patch).

Marco van de Voort wrote

He wants the source as a kind of DOM model. That is IMHO a valid use, but I doubt it is unifiable with the current parser.

Not nearly as complicated as a DOM model really. I just want to insert some timing calls and a new entry to the uses clause. See my reply to Michael to understand what I am trying to accomplish.

This is typically one of the reasons why the "one parser to rule them all" approach is flawed.

For fpGUI IDE, I hope to do more code modification/generation using fcl-passrc, so then I'll can answer that question. :)

Michael Van Canneyt wrote

It breaks the parser, more specifically NextToken. The parser ignores whitespace. It should also ignore the new tokens.

I attached a parser patch to that bug report in Mantis. I tested the scanner and parser over various units of mine, and it seems to work fine now.

Why ?
To the compiler there is no difference between EOL and whitespace.

If you want to generate it with the exact layout, then, yes. But for profiling I don't see the need.

BTW: fcl-passrc can't parse program units either, but I'm already working on a patch for that.

This is a known shortcoming.

Well, I still fail to see why you need the tab and EOL.

Michael.

Maybe to the compiler it doesn't matter, but to a human trying to debug their profiler to see if they inserted code in the correct place, or to see why code doesn't compile any more after the profiler modified the code - such "human readable code" is rather important. :)
If no NewLine characters are inserted into the newly generated units, a compiler error on line 1 doesn't help the developer much. Line 1 could be 50k bytes or more long.

Lazarus can't open the file (error message says it doesn't look like a text file), other editor show one line, but clips text because they can't display such a long single line of text. The profiler becomes a near perfect obfuscater. :-)

Well, I still fail to see why you need the tab and EOL.

Well, does it hurt having them tokenized?

If anybody other than me comes up with a new idea/way of using the fcl-passrc parser on tokenizer - maybe they would like to rebuild source code at some point. Without tokens telling you where EOL used to be or TAB characters used to be (though the latter is not to important), it does limit it's functionality.

Attached is a file processed with the profiler and fcl-passrc that doesn't use tkLineEnding or tkTab. It's rather hard to read and fix compiler errors introduced by the profile inserting code.

I don't see why introducing such new tokens is bad? It can only make the tokenizer and parser more useful in the long term.

mmm, looks to me that you are trying to implement some kind of AOP (Aspect Oriented Programming) for this. (which IS interesting)

The way most profilers that I've worked with did it was adding those timing-stuff (and counting) on a link-phase. (or through a custom exe-loader). Every PIC/method-table or VMT was replaced by a stubb. Or is it something else you want?

(custom attributes, the .net way, with AOP methods and a method sink?)

Anyway, if so, would this "profiling" stuff be optional, on/off all or on/off for a specific method/class/unit/package/program?

kind regards,
Dimitri Smits

Dimitri Smits wrote

Anyway, if so, would this "profiling" stuff be optional, on/off all or on/off for a specific method/class/unit/package/program?

fpprofiler (which I now switched to instead of my own) is optional. Profiling is applied per unit, and then "attaches" to every procedure/function or method in that unit. fpprofiler can be told to ignore certain units. How it currently works, is that you pass the exact same compiler parameters to the profiler, as you would to compile your project. It inserts the profiling code, compilers your project with the profiling code, then reverts all profiling changes. You end up with your "normal" code, but with a "fat" executable which does profile logging for you.

Reporting wise, it does timing, call count and generates a call graph. Output formats are Plain Text, LaTeX and PNG images for the call graph.

What I like is that it is really easy to get started, and the profiling information is nice and easy to understand and interpret. Valgrind, callgrind, cachegrind, coffee-grinder etc... just seems very complex to interpret. This will scare of any developer new to profiling.

Another BIG plus point for fpprofiler, is that it only requires FPC - no other dependency. Something I always like - low dependencies! :)

I'm planning on integrating fpprofiler (or a similar profiler) into fpGUI IDE (my sideline project), so you can enable it by a toggling a checkbox (just like you currently can for Valgrind or Gprof via a command line parameter). What I want to add extra into fpGUI IDE, is more visual screen that interpret the profiling data.

Anyway, I'm getting way off-topic here... I'll wait an see what Michael says regarding my last email and the new patch to fix the pparser unit.

If no NewLine characters are inserted into the newly generated units, a compiler error on line 1 doesn't help the developer much. Line 1 could be 50k bytes or more long.

The FPC scanner uses a flag (EolFound?), to signal an EOL to the preprocessor.

DoDi

Graeme Geldenhuys wrote

Well, does it hurt having them tokenized?

Of course.
It slows the code. Marginally, I agree, but nevertheless.

I never said it is bad; I just don't see why you need it.

You could perfectly do it differently, e.g. insert a newline after certain tokens:
then
begin
;
end
else
var
type
const

That would perfectly be enough for your purposes, which is, to debug your profiler. And once that is done, it is no longer 't worry, I'll apply the patch, but you have not convinced me it is a needed patch, even for your purposes. I'm surprised you didn't think of the above ways to do what you need.

Michael

2010/8/23 Michael Van Canneyt :

> I don't see why introducing such new tokens is bad? It can only make > the tokenizer and parser more useful in the long term. I never said it is bad; I just don't see why you need it.

I guess one wants the line numbers between the orginal source to be the same, for example to have the same line info in case of a backtrace when you compare before and after the changes.

So, programs compiled with profile info generate different backtraces with line info as programs without them. That is not nice.

That would perfectly be enough for your purposes, which is, to debug your profiler. And once that is done, it is no longer needed at all.

Why is it not longer needed?

Don't worry, I'll apply the patch, but you have not convinced me it is a needed patch, even for your purposes. I'm surprised you didn't think of the above ways to do what you need.

I think with the new tokens a profiler can do a better job.

Vincent

Op 2010-08-23 01:17, Michael Van Canneyt het geskryf:

> Well, does it hurt having them tokenized? Of course. It slows the code. Marginally, I agree, but nevertheless.

Sorry, but I disagree here. Before it got tokenized as a tkWhitespace; now it gets tokenized as tkLineEnding or tkTab. It's still being assigned a token, just one that describes it better. As for the parsers NextToken method now having two extra tokens it needs to ignore. I profiled that, and the difference was too small to notice.

Vincent hit the nail on the head. Now the compiled code with profiling has very different line numbers (now much more than before) to the real code. So line number info reported by LineInfo unit is now even more useless. And considering that under 64-bit systems, method names are not reported by LineInfo for some reason, the line number info is rather important.

needed patch, even for your purposes. I'm surprised you didn't think of the above ways to do what you need.

See two paragraphs back. ;-)

Regards,
- Graeme -

That's why I said 'marginally' :-)

This is IMHO a fallacy: the line numbers will change anyway if you insert code lines, so you must check the generated code with profiling in each case, not the original code. Therefor the information about the original lines is not needed.

Anyway, the point is moot; I'll apply the patch.

Michael.

Michael Van Canneyt wrote

Anyway, the point is moot; I'll apply the patch.

Patch applied, rev. 15877.

Michael.

Op 2010-08-23 09:44, Michael Van Canneyt het geskryf:

Patch applied, rev. 15877.

5 minutes after I got a trunk update and rebuild the whole FPC 2.5.1!

Thanks, Michael. :-)

Regards,
- Graeme -

Sorry for OT, but where can I find fpprofiler? I didn't manage to find svn link to it (some links google gave me were broken...). Even better would be to have your updated/improved profiler.

Thanks
Žilvinas

Op 2010-08-26 15:50, Žilvinas Ledas het geskryf:

Sorry for OT, but where can I find fpprofiler? I didn't manage to find svn link to it (some links google gave me were broken...). Even better would be to have your updated/improved profiler.

It's in the 'fpcprojects' repository.

http://svn.freepascal.org/svn/fpcprojects/fpprofiler

I'm going to move fpprofiler history, including my changes to a Git repository. Not sure where yet, probably SourceForge or GitHub. 'fpcprojects' repository is a mess, because it's one SubVersion repository for many projects - so history is all mixed up. I already extracted all the fpprofiler specific history from the SubVersion repository into a local git repository, I simply need to push it to a new home.

Regards,
- Graeme -

Graeme,

Can fpprofiler be used to profile code in a shared library (e.g. DLL on Windows or .so on Linux)? If so, then I'd be very interested in looking at it as well, and giving it a go.

Thanks much,
Alan Krause

Graeme Geldenhuys wrote

It's in the 'fpcprojects' repository. http://svn.freepascal.org/svn/fpcprojects/fpprofiler

Thanks, I'll try it.

Please, post a link to it when you have it. I am definately interested in it!

Regards
Žilvinas

Op 2010-08-26 16:20, Žilvinas Ledas het geskryf:

> It's in the 'fpcprojects' repository. > http://svn.freepascal.org/svn/fpcprojects/fpprofiler > Thanks, I'll try it.

Just remember, that's not going to compile.

Please, post a link to it when you have it. I am definately interested in it!

I'll find it a new home tomorrow and post the link.

Regards,
- Graeme -

Op 2010-08-26 16:19, Alan Krause het geskryf:

Can fpprofiler be used to profile code in a shared library (e.g. DLL on Windows or .so on Linux)? If so, then I'd be very interested in looking at it as well, and giving it a go.

Yes, it should be able too - as soon as fcl-passrc can parse a library unit. I believe Dimitry is working on that now. I saw Dimitry committed yesterday a fix for fcl-passrc to parse a program unit. So we are almost there. :)

Regards,
- Graeme -

Yes, I understand that.

> Please, post a link to it when you have it. I am definately interested > in it! I'll find it a new home tomorrow and post the link.

If you will do it tomorrow then I will wait for it ;) BTW, what is required/preffered version of fpc? I didn't updated it in a few months (2010/06/09) so I guess I should get an update to use your improved version of fpprofiler?

Regards
Žilvinas

Op 2010-08-26 16:31, Žilvinas Ledas het geskryf:

BTW, what is required/preffered version of fpc? I didn't updated it in a few months (2010/06/09) so I guess I should get an update to use your improved version of fpprofiler?

I was testing with the latest FPC Trunk (2.5.1) to get the latest features of fcl-passrc. I don't know if fpprofiler will work on 2.4.x - I haven't tried.

Regards,
- Graeme -

Graeme Geldenhuys wrote

I'll find it a new home tomorrow and post the link

Any news? ;)

Regards
Žilvinas

2010/8/27 Žilvinas Ledas:

> I'll find it a new home tomorrow and post the link Any news? ;)

Yes, I placed in on GitHub. You can clone the repository as follows:

via git protocol (faster):
git clone git://github.com/graemeg/fpprofiler.git

via http protocol:
git clone http://github.com/graemeg/fpprofiler.git

Alternatively (though not tested) - you should be able to download a zip archive of the latest code via this URL:
http://github.com/graemeg/fpprofiler/zipball/HEAD

NOTE:
Make sure the code you want to profile is backed up, or in some repository. I have found cases where the profiler (fpp) failed to modify all units due to parsing errors, then failed again restoring all units to what they were. I think I fixed this bug, but I thought I better let you know.

Thank you - I will try it today or tomorrow. I will write if I'll find some bugs.

Regards
Žilvinas

Graeme Geldenhuys wrote

Alternatively (though not tested) - you should be able to download a zip archive of the latest code via this URL: http://github.com/graemeg/fpprofiler/zipball/HEAD

Tried sample project today. Some a comment and a question:
1) I have a strange error when the same file is modified twice (and afterwards restored twice). One is:
insert: E:\lazarus\components\fpprofiler\demo\test001\test001.pp .......... OK
and another is:
insert: .\test001.pp
- After these two modifications modified units have compilation errors (duplicate uses units).
- When modifications are removed - duplicate files are deleted totally.
- (BTW, when -r is used for fpp it is passed to fpc and fpc is not happy.)
- I am attaching console output if it I used a workaround: In procedure InsertProfilingCode(FileList: TStrings; ModTokenProc: TModTokenProc);
-----

   //make a copy of the original files and process them
   for i := 0 to FileList.Count - 1 do
   begin
     if FileExists(FileList[i] + FPPROF_EXT) then begin  // <-- if the

file is processed (backup exists) - do not process it again

       writer.AddIgnoredFile(FileList[i]);  // <-- (don't know if this

is required?)

       Continue; // <--
     end; // <--

-----
In procedure RemoveProfilingCodeFromFile(const FileName: string); -----
begin
if FileExists(FileName) then begin // <-- if there is no backup file
- do not delete original file
-----

2) Where should I write comments/bug reports for fpprofiler?

Regards
Žilvinas

P. S. I know I didn't have to include
-FuE:\lazarus\components\fpprofiler\fpprof\
This log is from an experiment...

2010/8/28 Žilvinas Ledas :

Tried sample project today. Some a comment and a question: 1) I have a strange error when the same file is modified twice (and afterwards restored twice). One is:

A known problem. I had a partial fix, but not 100% yet.

fpp is modifying the same file twice. You probably passing in a parameter as follows.

-Fu/path/to/unit/ <--- test001.pp -Fu. <-- also points to test001.pp

fpp only looks at paths and processes everything inside each, hence it could process some directories (reference differently) twice. I simply removed one of the duplicate -Fu parameters to resolve this. But ideally, as you noticed, this needs to be prevented by fpp.

2) Where should I write comments/bug reports for fpprofiler?

I guess fpc-users mailing list, or maybe email me directly. Darius (original author of fpprofiler) contacted me in private - I'll discuss with him what is best path to follow for future. His idea was to one day include fpprofiler with FPC.

Žilvinas Ledas wrote

Tried sample project today. Some a comment and a question: 1) I have a strange error when the same file is modified twice (and afterwards restored twice). One is:

I'm looking at this issue atm. As soon as I have a definite solution I will commit.

2) Where should I write comments/bug reports for fpprofiler?

Please put up a bug report in mantis and assign it to me. I have updated SVN with all patches Graeme has sent me, so we should use this from now on.

I have also create a separate wiki page:
http://wiki.freepascal.org/fpprofiler. Feel free to add screen shots and more information.

Regards, Darius

fixed in rev2491