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

It is not possible to use JavaSymbolSolver in a multithreading context #2671

Open
corradomio opened this issue May 14, 2020 · 7 comments
Open
Labels
Improvement Not a bug, but a way that JP can be be enhanced to work better. Thread safety / Multithreading

Comments

@corradomio
Copy link

The current implementation of JavaSymbolSolver can be NOT used in a multithreading application. It is necessary to reimplement some classes and to control the access to some internal data structures.

The main problem is that it uses too memory: this because a type resolver is a dictionary and it is necessary to create the hierarchy of type solvers for each source code analyzed. BUT the content of this dictionary is the same for ALL source codes analyzed in parallel.

It is necessary to separate the dictionary from the typesolver: multiple type solvers, same dictionary. For example the JDK is the same for ALL.

Another problem is JavaParserFacade.get(): synchronizid this method we introduce a point of synchronization that can be avoid.

This is only the list of the major problems found.

@MysterAitch
Copy link
Member

I see that the goal is to deprecate and remove JavaParserFacade in favour of using : #1377

More than happy to have improvements made until that happens though :)

Modules and multi-threading are two of my weakest areas, so some extra support from you on these would be very gratefully received.

Personally I'm a fan of the idea supporting multi-threading, I'm just not sure what is needed to make it happen! Hopefully then we can also run the test suites in parallel thus much faster!! :)

#2671
#2668

@MysterAitch MysterAitch added the Improvement Not a bug, but a way that JP can be be enhanced to work better. label May 14, 2020
@MysterAitch
Copy link
Member

A reply I got on chat when I asked for comment about this issue is that we would usually advise not using JP across threads whenever similar questions are asked (somewhat paraphrasing here -- note that I'm still quite new and @matozoid has been around a lot longer than I have, so I'm inclined to trust him more than I trust my own views here!).

My presumption is that it would be a substantive amount of work to do well before we could have "thread-safe" being something that could actually be used to describe JavaParser. Or maybe this is an easy/trivial thing to do for someone who knows what they're doing? At the risk of being annoyingly open (and repetitive 😉) about pointing this out, personally I don't know enough to say one way or the other.

Given that you have clearly gone further down this route than I have and it sounds like you are already using a modified/patched/extended version that is able to be used across threads (pending #2668), would you mind sharing some details about this and what would be involved if we were to actively pursue a thread-safe JavaParser?

perhaps there is a distinction to be made between making JP thread-safe, versus something that actively blocks/breaks other people trying to add a thread-safe wrapper/extension to JP?

@corradomio
Copy link
Author

corradomio commented May 15, 2020 via email

@corradomio
Copy link
Author

corradomio commented May 15, 2020

At the moment, "JavaParserFacade.get()" IS ""the problem""because

  1. it is a GLOBAL/STATIC method
  2. it caches ALL type solvers
  3. it is used in several places inside the library
  4. it is NOT synchronized

I tried to remove it, but I have not implemented this part.
The current ""possible"" solution is

  1. to create a JavaParserFacadeFactory that contains the map typesolver -> JavaParserFacade (the current static part of JavaParserFacade inserted in a separated class)
  2. to extend TypeSolver with a method "getFactory" that return a JavaParserFacadeFactory >

The call (repeat, used in the library implementation)

JavaParserFacade.get(typeSolver)

it will converted in

public static JavaParserFacade get(TypeSolver typeSolver) {
    return typesolver.getRoot().getFactory().get(typeSolver)
}

In this way, the root typeSolver contains the ""thread-local"" cache of type solvers, and with a minimum impact on the current code. The problem is to change the current typeSolver interface.

But I am not sure.

@corradomio corradomio changed the title It is not possible to use JavaSymbolResolver in a multithreading context It is not possible to use JavaSymbolSolver in a multithreading context May 15, 2020
@MysterAitch
Copy link
Member

It'll take me a few more read-throughs before I can properly understand, and then intelligently reply to, what you have written...

First though, I wanted to drop a note sooner rather than later to say that I really appreciate the detailed and clearly well thought out comments -- thank you!

@maartenc
Copy link
Contributor

maartenc commented May 19, 2020

What is the problem you encounter in a multithreaded environment?
I'm using the JSS on to analyse all our projects (the biggest one has 60000 source files) with 3 concurrent threads. I didn't notice any threading-issue yet.

Maarten

@corradomio
Copy link
Author

corradomio commented May 19, 2020

I have already described the problems in the previous posts.
SymbolSolver NOT JavaParser

  1. memory used by type solvers
  2. WeakHaskMap (used in JavaParserFacade.get(...) USED INSIDE JSS) that, when updated in parallel, creates loops in the internal linked list handled by ""WeakHaskMap.table"". Method: "WeakHaskMap.get(...)". In Java 8 source code, file WeakHaskMap.java, line 403.
public V get(Object key) {
        ...
        Entry<K,V> e = tab[index];
        while (e != null) {
            if (e.hash == h && eq(k, e.get()))
                return e.value;
            e = e.next;              <========
        }
        return null;
    }

JavaParser/SymbolSolver 3.15.21 & 3.15.22. Obviously I have not idea if previous versions of the solver have the same problem or not.

I am using 8, 20 or 64 thread (it depends on the hardware used).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Not a bug, but a way that JP can be be enhanced to work better. Thread safety / Multithreading
Projects
None yet
Development

No branches or pull requests

3 participants