Looks good to me in general. I'm not familiar with the peg parsers - I assume these are generated and one has to always run `make` before so the sources get created, right?

I'm not familiar with the peg parsers - I assume these are generated and one has to always run `make` before so the sources get created, right?

Yes, they are not committed in the git, but they are generated by `packcc` as needed. At the very least `make peg/kotllin.c` should be called, otherwise the update script fails. Should I add the call in the script?

@dolik-rce pushed 1 commit.

a696977aee3267154bb42cc7d84e44b677849835 follow ctags directory structure, call make before copying peg parsers

@dolik-rce commented on this pull request.

•    shutil.copy('parsers/' + f, dstdir + '/parsers/' + f)
  

- elif os.path.exists(f): - shutil.copy(f, dstdir + '/parsers/' + f) - else: - print('Error: Could not find file ' + f + '!') - sys.exit(1) + shutil.copy(f, dstdir + '/parsers') + +os.chdir(dstdir + '/peg') +peg_dst_files = glob.glob('*.c') + glob.glob('*.h') +peg_dst_files = list(filter(lambda x: not x.startswith('geany_'), peg_dst_files)) +os.chdir(srcdir + '/peg') +os.system('make -C {} peg/{}'.format(srcdir, ' peg/'.join(peg_dst_files))) +print('Copying peg parsers... ({} files)'.format(len(peg_dst_files))) +for f in peg_dst_files: + shutil.copy(f, dstdir + '/peg')

I was actually thinking that the script could use some "readability improvements". Like using functions and giving it some structure... But it seemed a bit out of scope for this PR, so I stayed with the current style.

Would you mind if I implemented it in follow-up PR?

I just noticed the size of the parser - 25klocs and 1MB of sources. Have you checked how much the size of stripped libgeany.so increases by this patch?

I just noticed the size of the parser - 25klocs and 1MB of sources. Have you checked how much the size of stripped libgeany.so increases by this patch?

Yes, the generated code is humongous. I guess it is price for implementing full Kotlin grammar. I have tried to check the library size with `make && strip --strip-unneeded`. Current master is about 3.57 MB, this branch yields 3.72 MB, so roughly 150kB. I agree it is quite a lot, but still manageable I hope.

Of more interest is how fast (slow) is it, remembering it runs while editing, will Kotlists get big pauses?

Frankly, the speed could be better. This has already have been discussed when I was adding the parser to ctags and it even resulted in some improvements in PackCC (which is used to generate the code), but it is still slower than handwritten parsers. You can have a look at https://github.com/universal-ctags/ctags/pull/2866 and https://github.com/universal-ctags/ctags/issues/2878 if you're interested.

Anyway, the pauses during editing are not noticeable for reasonably sized files. The only situation where the speed might be annoying is when user opens big kotlin project. But I'm not sure how this actually works, does Geany always process all open files on start? Or is there some kind of caching? I think some of the projects-related plugin even allowed to set option to index all files, even those that are not open. This could then take quite some time.

Implementing the whole grammar has problems for this use-case, not only speed, but also robustness. It is being used on files that are being edited and may be eronneous or incomplet.

Unless the parser can recover from an error it is going to lose all or many symbols in more cases if it parses things that are not essential to detecting the tags since it will then find more errors. It is a problem with many languages, not just kotlin (see for example #2916), but the fact many tag parsers skip lots of code reduces the impact of partly edited code (outside of declarations).

This is also why full fat IDEs to try to insist that at least the the structural syntax is complete (eg {}s match) so their parsers have some handles to resync and recover after errors.

Yes, the generated code is humongous. I guess it is price for implementing full Kotlin grammar. I have tried to check the library size with make && strip --strip-unneeded. Current master is about 3.57 MB, this branch yields 3.72 MB, so roughly 150kB. I agree it is quite a lot, but still manageable I hope.

It's not that bad, I guess I'm still stuck with the "it must fit a floppy" mentality.

<!DOCTYPE html>

Implementation | Time | Avg. per file | Tags added -- | -- | -- | -- C | 4 s | 66 us | 567695 RegExp | 155 s | 2.5 ms | 527908 Peg | 523s | 8.6 ms | 782500

<br class="Apple-interchange-newline">

This is quite surprising, I would have expected the performance to be somewhere between a hand-written parser and a regex parser but it probably depends on the grammar and the lookahead it has to perform. Have you tried profiling the parser to see if there's not some unexpected bottleneck? With hand-written parsers I used the following:

https://wiki.geany.org/howtos/profiling/gperftools

Unless the parser can recover from an error it is going to lose all or many symbols in more cases if it parses things that are not essential to detecting the tags since it will then find more errors. It is a problem with many languages, not just kotlin (see for example #2916), but the fact many tag parsers skip lots of code reduces the impact of partly edited code (outside of declarations).

To be fair, hand-written ctags parsers aren't that great at error recovery either - when you type an extra `{` at the beginning of a file, it won't get parsed.

This is quite surprising, I would have expected the performance to be somewhere between a hand-written parser and a regex parser but it probably depends on the grammar and the lookahead it has to perform. Have you tried profiling the parser to see if there's not some unexpected bottleneck?

I did a lot of profiling on PackCC and it turns out it is not very efficient with memory allocations (see https://github.com/universal-ctags/ctags/pull/2866#issuecomment-780332841), the backtracking is constantly allocating and deallocating small chunks of memory. So, more rules leads to more backtracking, which leads to more allocations, which slows it down. I have even [proposed couple possible optimizations](https://github.com/arithy/packcc/pull/48), but I did not have from enough manpower to actually implement it (sadly, I am not very proficient in C).

I might look into the grammar optimization in future, so the parser might become somewhat faster (especially, if it slows Geany down, that would annoy me a lot). Probably could beat the RegExp based ones (for comparable grammars). According to creator of ctags, the peg parser is about 15-30 times slower than handwritten parser (citation from https://github.com/universal-ctags/ctags/pull/2866#issuecomment-778786809). Even with grammar optimizations, it would still probably be about an order of magnitude slower than let's say current C parser.

That is really surprising :frowning_face: I'd expect RPi 4 to handle this much better. I don't have any slower hardware readily available right now, but I have tested it with `cpulimit -l 50 -i geany SQLExpressionBuilder.kt`, which on my i5 should mean it should get about 1.2GHz and was still pretty smooth. There might of course also be some difference due to memory speed.

Well, I started this as a "Christmas project" last year, so I guess I'll just have a another one for this year: to make it faster.

... seems to contain many anonymous functions and it seems to be a common pattern in Kotlin files and seeing the symbol tree filled with <lambda> doesn't seem that useful to me so maybe these shouldn't be reported to Geany.

Yes, lambdas can be quite common in Kotlin code and you're right, that they are probably useless in the symbol tree most of the time. I think the only valid reason to keep them is that there might be something else declared in their scope. Perhaps we could filter out all lambdas without any child nodes?

@dolik-rce pushed 2 commits.

9fe7ef7e8d26324c1a549f152796b0c1f7db066e Add support for Kotlin tags 8b695708aeafb641e17fc13e8f58d503aa547c0d follow ctags directory structure, call make before copying peg parsers

@dolik-rce pushed 1 commit.

7c66a56ce749b6e735d2a352549a31531f3a3315 fix meson build

@dolik-rce pushed 3 commits.

cb4be436d471b88be2e63f3f6032076dd8a0a608 Add support for Kotlin tags b052e116fdabc3996f18f8fd10c54d8fc3e18167 follow ctags directory structure, call make before copying peg parsers da3726e9d10b10df80fdc1994ac9ecabea7e5d3c fix meson build

@dolik-rce pushed 1 commit.

05d2472fcb399535648a556cdec8b4b5f9f8ba8c fix meson build

@dolik-rce pushed 0 commits.