Our vala setup is *very* low-tech right now. See plugins/vala-pack/ide-vala-index.vala

Who is in charge on Vala plugin?

Who can point to understand your implementations, in order to fix/improve them?

Well I wrote it, but my preference would be for someone more familiar with the vala project take it over. It is somewhat designed after how libclang works.

This is a great opportunity to improve both projects.

First all, in order to help better and reduce the time to invest to figure out how it works, I need to know:

* Plugin use Builder's libs with auto generated vapi files?

* How it gets system vapi files to parse over?

* How it gets/discover vapi in the project?

* A brief explanation about what is and how Ide.ThreadPool.push works?

maybe just a little explanation and pointing to related files should be enough.

(In reply to Daniel Espinosa from comment #4)

> * Plugin use Builder's libs with auto generated vapi files?
> * How it gets system vapi files to parse over?
> * How it gets/discover vapi in the project?

These can all be answered by looking over how we setup the Vala context in ide-vala-index.vala. There are comments/todos about this.

> * A brief explanation about what is and how Ide.ThreadPool.push works?

Simply using GTask is not sufficient in many cases due to the lack of control over the thread pool size and throttling on number of concurrent workers. So we have a threadpool that allows categorizing the workload so we can throttle as necessary. We still like the GTask API though, so we try to maintain the ability to use that helper object.

After checking code in Vala plugin, I found that, for example, for ide-vala-index.vala, and method:

async void add_files ()

Is declaring async, while this is implemented in Vala using GSimpleAsyncResult, this is, I think, a redundant threading approach. Just check C generated code.

I think we need to remove that and just use simple functions, calling Ide.ThreadPool.push() to execute asynchronously the code. Then for the example above this should be:

public void add_files (ArrayList<GLib.File> files,
  GLib.Cancellable? cancellable)
{
  Ide.ThreadPool.push (Ide.ThreadPoolKind.COMPILER, () => {
    lock (this.code_context) {
      Vala.CodeContext.push (this.code_context);
      foreach (var file in files)
        this.add_file (file);
      Vala.CodeContext.pop ();
    }
  });
}

In this case Ide.ThreadPool, will execute the above code asynchronously, but will be initiated just one thread. In the actual approach, initiate two: one for GSimpleAsyncResult and one for ThreadPool.

Along Vala plugin, there are most of this issues, this may mean lot of crashes because no carefully synchronised threats.

I would like to change this and propose to disable by default Vala plugin, until it is more stable.

What do you think?

One additional comment is about:

Why Ide.ThreadPool.push() is called using Ide.ThreadPoolKind.COMPILER? Is it better to called using Ide.ThreadPoolKind.INDEXER?

If COMPILER ThreadPool is used then, Are we pushing code in a pool used for indexing?

COMPILER was just the default I used. We can certainly look at adding another mini-pool for things related to administration. But all of the callbacks should be locking the code context before using it. Is there somewhere where you see access to the context not being synchronized?

As for using async, I don't mind dropping the use of it as long as we aren't relying on the timing of the completion (via yield) in other places. At least GSimpleAsyncResult uses a mini-thread-pool for the job workers, but yes, I'd like to get usage of it out of the code-base.

What I *really* want, is to add a GTaskScheduler to GTask and port Vala to use both.

Created attachment 326355 [details] [review]
Removed async to Vala plugin add_files() in index

This is save even as I found a bug on task cache.

I found a crash when loading vala documents, may is related to problems in async menthods,

To reproduce, I used GXml source code, then open different files, some times happend at second or third, or even after lot of files, but eventually hapends.

I'm using latest git and running under jhbuild.

This is the back trace:

+ Trace 236191

Now When using GXml source code, I see Builder can't parse build flags, then it can't find symbols for Gee.


15:20:13.0240                vala-pack-plugin[22162]:  WARNING: ide-vala-index.vala:210: Failed to extract flags from make output


You'll see some issues on building, because to stamp found, but I need to figure out how do you build.


make: *** No rule to make target 'libxml.stamp'.  Alto.


GXml use no libxml.stamp, but a vala-stamp: rule located at gxml/Makefile.am

(In reply to Christian Hergert from comment #8)
> COMPILER was just the default I used. We can certainly look at adding
> another mini-pool for things related to administration. But all of the
> callbacks should be locking the code context before using it. Is there
> somewhere where you see access to the context not being synchronized?
> 
> As for using async, I don't mind dropping the use of it as long as we aren't
> relying on the timing of the completion (via yield) in other places. At
> least GSimpleAsyncResult uses a mini-thread-pool for the job workers, but
> yes, I'd like to get usage of it out of the code-base.
> 
> What I *really* want, is to add a GTaskScheduler to GTask and port Vala to
> use both.


I found your for for GTaskScheduler at:

https://github.com/chergert/gtask/wiki

But what about to use async methods, as Vala have today, for Builder's Vala plugin?

Can I experiment with that, in order to avoid Ide GTask pool, and do asynchronous task? Is is acceptable?

(In reply to Daniel Espinosa from comment #10)
> I found a crash when loading vala documents, may is related to problems in
> async menthods,
> 
> To reproduce, I used GXml source code, then open different files, some times
> happend at second or third, or even after lot of files, but eventually
> hapends.
> 
> I'm using latest git and running under jhbuild.
> 
> This is the back trace:

For this problem, you need to open the files quickly like when a project is re-loaded with lot of files. Now I'm stoped, because I don't know how to remove recent files from being opened on project open. Please help me!

Update from git and see if things are fixed. I missed a CodeContext.push() inside a lock while adding the makefile parsing.

Also, I fixed a difference between non-recursive automake and recursive automake, which might help your libxml.stamp issue.

The main problem with using Vala's built in async is that GSimpleAsync pushes everything onto the I/O job queue in Gio (which also has a very finite thread pool size and contents with actual read/write I/O). It should be a priority to fix that in Vala because its a bad idea to use it as such.

That said, we are already clearly "doing the wrong thing" by having two async work items ...

If we can get Vala to move to GTask, it should be possible to have a GTask that completes that never schedules any code. (So we'd only have the 1 extra object allocation overhead when using Ide.ThreadPool).

Is aceptable to remove, for now, Ide.Ide.ThreadPool and just use Vala asyc methods?

I have a branch implementing above. It seems to work decently and could be the right way while Vala implements GTask. Then just an update should be enough.

(In reply to Daniel Espinosa from comment #16)
> Is aceptable to remove, for now, Ide.Ide.ThreadPool and just use Vala asyc
> methods?

It is not, I want thing using Ide.ThreadPool.

For example, we have debugging tools that give us insight into tracking down runtime issues with counters. Not using the things we built for exactly this type of problems just makes it harder later on.

/usr/libexec/gnome-builder/ide-list-counters <pid> for example, gives us insight into the running gnome-builder instance. Managing and throttling threads is an important aspect to keeping the IDE fast and interactive latency down.

We have no insight into the GIOScheduler that GSimpleAsyncResult will use, not to mention it will contend with threaded I/O operations which we are doing all the time (and share that scheduler).

OK. Then what we need is to avoid using Vala async methods in all code and add  a few Ide.ThreadPool.push() around when async is required.

I just took a look through the generated C code for ide-vala-index.vala. I don't there is anything to worry about.

While the vala code generator creates a new GSimpleAsyncResult, it doesn't schedule it on the GIOScheduler, nor does it generate *any* threaded work. It simple creates 1 additional GAsyncResult object that we wouldn't otherwise need.

It's really not the end of the world when compared to say, queuing a work item onto a thread pool.

The thing I dislike most about both GTask and GSimpleAsyncResult is that they schedule an idle callback for delivery creating a new GSource instead of (re)using an existing permanent source. But again, not the end of the world.

So I'd just keep using "async" as usual and thread-pool push.

I've switch to master updated from git. Problem remains about Comment 10, same backtrace.

I noticed a message:

vala-pack-plugin[11006]:  WARNING: ide-vala-index.vala:210: Failed to extract flags from make output

From GXml sources, I'm using "vala-stamp" as a rule to compile, because I always convert to C then compile.

Maybe this is making not to find all switch to include from, and for commit:

https://git.gnome.org/browse/gnome-builder/commit/?id=cb818ac6f7c955d1bb25413c0289ad77c5e1014d

Your target stamps use target name with sufix ".stamp" or "_vala.stamp", I would like to know if this is autotools standard I need to follow, or I need to switch my code to use that stamp naming conventions, in order to be usable in Builder or there are a way to say to builder witch stamp name should it search for.

Even on flags, GXml uses a general AM_VALAFLAGS variable, is it supported? because GXml mixes C and Vala code, then it translate all .vala to .c, before to compile.

More important, following error continue, this should not be present even if no target and flags could be located.



Backtrace:

+ Trace 236194

See if commit fed3631b486689cb240e14b49abfdda1c8cc9571 fixes your crash.

As for the stamp question, it is really expensive to go through and try to guess any possible stamp names (we have to parse 30mb of text, including state, and execute make subprocess for more info). So what we do is look for the two stamps we know that automake uses based on the translated target name. (We have to look for the .lo which will depend on the .c, not the .vala).

If we missed some, we should add support for those too.

But going through all possible targets and trying to guess if "something.stamp" is the one we need is probably not worth the effort. Where as instead we should just inform people that if they don't use the built-in _SOURCES support automake, to name their stamps similar to that of automake.

In this case, perhaps change vala-stamp to libgxml_0_10_la.stamp which would simultaneously allow gxml to move towards non-recursive automake should that become desirable.

I think commit 4c71cc91207972bc4261e540eac1e49bca62c91b will also help a bit at extracting more flags from the gxml build system without changes.

I'm starting to think that this an issue from different source, but with today master, I have no advances on remove this crash:

Back trace:

+ Trace 236196

Even using a different project (internal sorry), I can open it, see symbol list, autocompletion (just for common files), and so, but even that simple project, I close almost all opened documents to have it usable next time it is open.

Please tell me how can I prevent loading recent files opened in a project, because that makes me fail to load GXml project.

With all these problems, may we need to disable Vala pack by default.

(In reply to Daniel Espinosa from comment #26)
> Please tell me how can I prevent loading recent files opened in a project,
> because that makes me fail to load GXml project.

We don't have a toggle for that.

The magic happens in libide/ide-context.c:1884 (restore_in_idle).

We could add a gsetting to org.gnome.builder and appropriate preference in libide/preferences/ide-preferences-builtin.c

I still see the nasty mapped file criticial in the log, but i can open gxml now while it restores files at load.

(In reply to Christian Hergert from comment #28)
> (In reply to Daniel Espinosa from comment #26)
> > Please tell me how can I prevent loading recent files opened in a project,
> > because that makes me fail to load GXml project.

I've added a gsetting to disable loading of previous files.

$ gsettings set org.gnome.builder restore-previous-files false

There is also a toggle in the Projects preferences section.

I've pushed this to master and to gnome-builder-3-20 branch so we'll see it in 3.20.4 when that is released later this week.

Created attachment 327363 [details] [review]
vala: add vala versioned and unversioned vapidir

This ensures we have the vala vapidirs such as:

  /usr/share/vala/vapi
  /usr/share/vala-0.32/vapi

available to the code context.

We certainly have more vala bugs to shake out, but we should make new bugs
for those issues. This should at least solve the bug as titled here, which
is missing versioned/unversioned vapidirs.

Attachment 327363 [details] pushed as dc10e0a - vala: add vala versioned and unversioned vapidir