Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

time taken to compile information #147

Closed
YashasSamaga opened this issue Feb 5, 2017 · 46 comments
Closed

time taken to compile information #147

YashasSamaga opened this issue Feb 5, 2017 · 46 comments

Comments

@YashasSamaga
Copy link
Member

YashasSamaga commented Feb 5, 2017

It would be useful if the compiler would display time taken to compile after a successful compilation so that we can optimize the code (preprocessor code?) for compile time.

Compiled in 5.344 seconds.

If possible, it would be better if the compiler can tell how much each part of the compiling process took separately.

If needed, a new compiler flag can be added to show the time information.

I am not sure how someone can get the time taken accurately as there is no standard time library in C which provides a time resolution of milliseconds. The only way out is to use clock() but that is a dirty and messy function.

@Dayvison
Copy link

Dayvison commented Feb 5, 2017

I no have idea what this guy do, to make the pawn so fast.
http://gtaforums.com/topic/760017-relsa-samp-addon/page-9#entry1069208196

Reply from author:
this version based on c++14, I don't think Zeex will merge speed changed

@YashasSamaga
Copy link
Member Author

added the time code: YashasSamaga@c98411c
fixed a bug: YashasSamaga@c6b956e

@Y-Less
Copy link
Member

Y-Less commented Feb 6, 2017

You really shouldn't be optimising for compile times. I can still see compile time information being interesting, but if you are sacrificing your code (speed/readability/repetition/etc) for compile times quite frankly you are doing it wrong - compiles are done offline because they are slow.

@YashasSamaga
Copy link
Member Author

I obviously wouldn't give away readability or runtime performance to reduce the time taken to compile. When a part of the code slows the compiler down awfully lot, I'd actually try another method.

@SysadminJeroen
Copy link
Contributor

I thought that files from the normal compiler and fast Russian compiler were the same, appears like they are not the same.

kdiff3_2017-02-06_22-23-16

Compiling the gamemode I'm working on takes about 30 seconds using the official compiler, and about 3 seconds using the Russian compiler.

Would it be better to compile with the Russian compiler to debug, and to compile with the normal compiler for production?

I'm saving so much time due to it normally taking 30 seconds to compile code, which can now be done in a couple seconds.

@kurta999
Copy link

kurta999 commented Feb 6, 2017

My friend (i'm not scripting anymore) using russian version for my mode and it works without any problem. I suggest you to test it and you'll see.

@SysadminJeroen
Copy link
Contributor

I am using the Russian compiler, but what I'd like to know what the downsides to that compiler is.
I have already used it.

@Zeex
Copy link
Member

Zeex commented Feb 7, 2017

I usually use an editor that tells me how much time it took to run the compiler, your editor could probably do the same with some addon/script. But that's if you simply want to know the total time.

As for the fast compiler: I haven't tried it but does it compile something as complex as YSI as fast?

@SysadminJeroen
Copy link
Contributor

@Zeex Could also use something like Linux's time binary to time how long execution takes.

And I'm not sure about YSI, but it does compile the following includes very fast.

#include <fuckcleo>
//include to load all defvars here
#include <fixes>
#include <timerfix>
#include <a_mysql>
#include <zcmd>
#include <irc>
#include <streamer>
#include <sscanf2>
#include <mSelection>
#include <FloodControl>
#include <SKY>
#include <weapon-config\weapon-config>
#include <SAMP-GeoIP\geolocation>
#include <samp-git-hash\git-hash.inc>
//include that includes all gamemode crap here

@Y-Less
Copy link
Member

Y-Less commented Feb 7, 2017

I'm not sure how it handles YSI either, though I don't like the number of warnings it spits out for legitimate code (including amx_assembly). They seem to have decided that recursion should be an enabled by default warning, which is a) a significant behaviour change from the old compiler and b) wrong, because recursion is a legitimate technique and warning for that is like warning that you have loops. However, regardless of the invalidity of warning for correct code, having it enabled by default is a serious issue because it means that it can't be avoided in library code - if I don't put the #pragma disablerecursion in the new divergent compiler will give warnings. If I put the pragma in, the old compiler will give an error because of the unknown pragma. I also don't know if the pragma can be enabled/disabled repeatedly so that known good code can keep using recursion without warnings but still have them enabled elsewhere. I may have to play about with combinations of that, version checks (does it even have a different version to test against for using the pragma), and #pragma warning.

@Y-Less
Copy link
Member

Y-Less commented Feb 7, 2017

Well I started doing some testing, and discovered that not only have they added new warnings, but they also removed the warning numbers from the error messages (why?) so I can't even see what number they've assigned to that stupid recursion warning to try and disable it - trial and error it is!

@SysadminJeroen
Copy link
Contributor

@Y-Less I had the problem with the recursion as well. That was about 2 days ago. I didn't think much of it other than it being odd.

@Y-Less
Copy link
Member

Y-Less commented Feb 7, 2017

While I don't think you should warn for legitimate code, the main problem is the lack of portability. It claims to be based on Zeex's compiler, but invented a new #pragma to disable just that one warning instead of using the existing #pragma warning. I have figured out, though, that this is the only way AFAIK to deal with it:

#if __Pawn >= 0x30A
	#pragma warning push
	#pragma warning disable 207
	#pragma disablerecursion
	#pragma warning pop
#endif

If you are on the original compiler, the whole block is skipped by the #if. If you are on Zeex's compiler, the new #pragma is not recognised, but that's only a warning and we can ignore that. If you are on the fast compiler, the recursion warning is fully disabled (and can't, as far as I can see, be re-enabled as it could have been using #pragma warning - I couldn't figure out what number it was and I tried a lot).

@Y-Less
Copy link
Member

Y-Less commented Feb 7, 2017

Also, the version reported by the compiler after a build is 3.2.3664, but __Pawn is 0x30A.

@Y-Less
Copy link
Member

Y-Less commented Feb 7, 2017

I'll add that blob to y_compilerpass, and amx_assembly.

@SysadminJeroen
Copy link
Contributor

@Y-Less, wouldn't it be better if only the specific version of the fast compiler would match the #if?

I'm assuming that they won't release a new version of the fast compiler.

@YashasSamaga
Copy link
Member Author

YashasSamaga commented Feb 7, 2017

If there is no warning number, then it isn't a warning. AFAIK you cannot issue a warning without giving the warning number (the function used to trigger a warning in the compiler code is error(warn_or_err_number, format, ...); the warning number and message is added by the error function automatically.

Whoever added that must have used pc_printf (or printf which is even worse) which isn't the correct way to issue a warning. You can confirm it by looking for "%d Warnings." at the end of the compiler message window. You should either get no total warnings message or the total warning count given must be one less than the actual count.

@Y-Less
Copy link
Member

Y-Less commented Feb 7, 2017

@Jeroen52: Yes it would, but I don't know how to just select that one compiler since it gives the same version number as the other one.

@YashasSamaga:

#include <a_samp>

forward MyFunc() <sss:hjk>;

MyFunc()
{
	MyFunc();
}

main()
{
}

Output:

F:\Gaming\SA-MP\gamemodes\warning_test.pwn(7) : warning 231: state specification on forward declaration is ignored
F:\Gaming\SA-MP\gamemodes\warning_test.pwn(9) : warning: recursion in MyFunc
Disable recursion warning: #pragma disablerecursion
Pawn compiler 3.2.3664	 	 	Copyright (c) 1997-2016, ITB CompuPhase


2 Warnings.

They've copied the warning: description format and add the result to the warning count, but haven't given it a number. At least they haven't removed all the other numbers as I previously thought they had.

@YashasSamaga
Copy link
Member Author

You can find their complete warning message at 0x46b62d in .rdata
http://prntscr.com/e5o3bj

So yeah, they broke the rules of issuing warnings.

@Y-Less
Copy link
Member

Y-Less commented Feb 7, 2017

It would be very useful if that version was open-source so we could see how they got the speed and smooth out some of the quirks.

@SysadminJeroen
Copy link
Contributor

@Y-Less: I have already sent an email a while ago, but haven't gotten a response yet.
chrome_2017-02-07_23-58-33

@karimcambridge
Copy link

Holy shit my compile time went from ~120s to 12.~s

tf?

Lol, nonamenoname (SPAWN_METAL) said that he uses c++14 features in his version.

@YashasSamaga
Copy link
Member Author

YashasSamaga commented Feb 11, 2017

C++14 is just a library, I don't think switching libraries make a significant difference. Moreover, Zeex might go against the idea of using C++14 because the code currently follows the C89 standards.

@justnonamenoname
Copy link

justnonamenoname commented Feb 11, 2017

fixed crash with #warning

now it write about #pragma compat 0 if you have error with y_hooks function define

gta-samp.com/files/pawncc.exe

now it possible to disable recursion search with
#pragma warning disable 238
it's bad idea to use error number bcz it possible zeex will use this numbers

Also recursion from ziggi is wrong, it's mark nonrecursion functions

warnings 217 (loose indentation) 203 (symbol is never used) & 204 (symbol is assigned a value that is never used) disabled as warning, only message.

I changed this for VSCode, different sounds for no bugs/warnings/error. this warnings not bugs in code at all

compat mode is enabled by default
If you compile mod after standart compile you wiill see symbol already defined, not error about several includes problem, it seems like compiler has problems
use #pragma compat 0

also there is other changes in compiler

amx hex must be same as from this ver (not original from sa-mp.com)

@Jeroen52
There is no email server on gta-samp.com

@Y-Less
I can change version to other, but it's other branch not ver, have no idea with ver num if you still need this

@karimcambridge
it's compile as cpp not c
unordered_map, unordered_set & std::string

@Zeex didn't answer about optimise on email & for thread #120

@Y-Less
Copy link
Member

Y-Less commented Mar 19, 2017

Symbol is never used is a mistake though, so should be a warning. If you know that a symbol is not being used theres both stock and #pragma unused to mark it as such. If a symbol doesn't use either of these and still isn't used - there's a mistake somewhere in your code. Either the function should be used and isn't being; or is ignorable but not marked as such. YSI Langs_Add for example relies on that warning to alert people to the fact that if they don't call that function they are using the library wrong. It is not stock because it MUST be called, and not doing so is most definitely a mistake, thus a warning.

@justnonamenoname
Copy link

you still see this text
just without word warning

@oscar-broman
Copy link
Contributor

@justnonamenoname Could you share the source code for it? It would make things much easier for everybody.

@oscar-broman
Copy link
Contributor

It seems @justnonamenoname likes to take what others share open-source, but keep his own modifications to himself.

Perhaps someone could follow the list of instructions he gave and try re-creating it? I don't know about you guys, but I personally don't like downloading binaries from sources I don't trust.

@Daniel-Cortez
Copy link
Contributor

Daniel-Cortez commented Oct 13, 2017

I'm going to try using std::unordered_multimap for symbol storage to speed up the lookup process. Transitioning the whole project to C++ would probably take a lot of effort (and I really doubt such changes would be accepted here), so would it be ok to make a wrapper for unordered_multimap as a separate library in C++ and statically link it to libpawnc?

@Y-Less
Copy link
Member

Y-Less commented Oct 13, 2017

Could you not find an open-source C map library? Would make it simpler to get integrated and merged without porting.

@maddinat0r
Copy link
Contributor

I already kinda did this here https://github.com/maddinat0r/pawn/commit/196fe1935239d71a9f2e91d1fac324ae6a6290fc.
It's not quite finished, as the map doesn't allow symbols with the same name to be added (thus generating compile warnings/errors in some cases).
Performance improvement (with an additional performance tweak) is not as high as with the Russian compiler, but there's another bottleneck somewhere related to symbol lookup.

@justnonamenoname
Copy link

justnonamenoname commented Oct 15, 2017

#120
Zeex ignored this at all

so

www.gta-samp.ru/files/PawnSource.rar
studio 2013

@YashasSamaga
Copy link
Member Author

Why shouldn't we migrate to C++?

@Southclaws
Copy link
Collaborator

@SysadminJeroen
Copy link
Contributor

@YashasSamaga Why migrate to a whole new language because of ??? while we can introduce new bugs and possibly even break compatibility.

@YashasSamaga
Copy link
Member Author

@Jeroen52 We don't have to change all the C code to C++. We could just start compiling the project with a C++ compiler and use the C++ standard library.

@Southclaws
Copy link
Collaborator

That's what maddinat0r is doing on his fork as far as I am aware. He said he managed to get his fork to run as fast as the Russian version.

@SysadminJeroen
Copy link
Contributor

@Southclaws Are there any trade-offs?

@Southclaws
Copy link
Collaborator

Not as far as I know, it's a proper fork of zeex' compiler maintaining the same source structure and proper warning handling so once it's ready, I'd advocate the use of it over the Russian version.

@SysadminJeroen
Copy link
Contributor

SysadminJeroen commented Oct 15, 2017

I certainly will if it is backwards-compatible, however we must keep the number of different forks to a minimum and try to merge the new changes into Zeex/pawn.

@Southclaws
Copy link
Collaborator

Southclaws commented Oct 15, 2017

Yes that's our hope, but since maddinat0r's is a proper fork using GitHub it'll be much easier for zeex to review and merge with master. I don't think anyone wants a diverged fork since zeex' compiler is currently the de-facto standard for advanced Pawn users afaik.

@SysadminJeroen
Copy link
Contributor

@Southclaws I can't wait for the new pull request.

@SysadminJeroen
Copy link
Contributor

Okay, so I did find some possible incompatibilities with the Russian compiler and Zeex' compiler.

It is a bit of code that does work on the Russian compiler, but not on Zeex' compiler.

However it could be due to my shitty programming.

@Southclaws
Copy link
Collaborator

I'd advise conforming to Zeex' compiler because it's widely used and well documented. If you write code that only works with the Russian one you're risking locking your code to that compiler only and if a major bug is discovered in the code, you'll need to do a bunch of extra work just to switch back. Also, it'll make getting help from others a lot more difficult.

@SysadminJeroen
Copy link
Contributor

That is exactly what I'm doing, not only that but my Continuous Integration uses Zeex' compiler, so I was surprised when code I wrote on the Russian compiler didn't work when the gamemode was automatically updated.

@justnonamenoname
Copy link

compat=1 default

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests