[m-rev.] for review: Use a thread pool to manage threads on the Java backend

Paul Bone paul at bone.id.au
Thu Jul 31 18:18:23 AEST 2014


On Wed, Jul 30, 2014 at 03:11:08PM +1000, Peter Wang wrote:
> On Tue, 29 Jul 2014 21:54:44 +1000, Paul Bone <paul at bone.id.au> wrote:
> > On Tue, Jul 22, 2014 at 02:52:03PM +1000, Peter Wang wrote:
> > 
> > > > diff --git a/java/runtime/MercuryOptions.java b/java/runtime/MercuryOptions.java
> > > > new file mode 100644
> > > > index 0000000..6f4c099
> > > > --- /dev/null
> > > > +++ b/java/runtime/MercuryOptions.java
> > > > @@ -0,0 +1,72 @@
> > > > +//
> > > > +// Copyright (C) 2014 The Mercury Team 
> > > > +// This file may only be copied under the terms of the GNU Library General
> > > > +// Public License - see the file COPYING.LIB in the Mercury distribution.
> > > > +//
> > > > +
> > > > +package jmercury.runtime;
> > > > +
> > > > +import java.util.List;
> > > > +
> > > > +/**
> > > > + * Process the MERCURY_OPTIONS environment variable.
> > > > + */
> > > > +public class MercuryOptions {
> > > > +    
> > > > +    private static MercuryOptions instance;
> > > > +
> > > > +    /**
> > > > +     * Get the singleton instance
> > > > +     */
> > > > +    public static synchronized MercuryOptions getInstance()
> > > > +    {
> > > > +        if (instance == null) {
> > > > +            instance = new MercuryOptions();
> > > > +            instance.process();
> > > > +        }
> > > > +
> > > > +        return instance;
> > > > +    }
> > > 
> > > MercuryThreadPool is already a singleton class so MercuryOptions doesn't
> > > need to be a singleton class itself.  You could remove some code.
> > 
> > What if other things add options to MercuryOptions in the future?
> 
> I disagree with adding code "just in case" when it is an internal detail
> that we can change freely.
> 
> > Or
> > parhaps we should introduce a class/make a class responsable for the handles
> > of all the singleton objects.
> 
> JavaInternal seems to be the place for that.
> I think keeping much of the global state together is a good idea.
> 

Okay cool,  I'll move the instances of MercuryThreadPool and MercuryOptions
into JavaInternal.

> > > > +                } catch (Throwable e) {
> > > > +                    // Some other error occured. bail out.
> > > > +                    System.err.println("Uncaught exception: " + e.toString());
> > > > +                    System.err.println(e.getMessage());
> > > > +                    e.printStackTrace();
> > > > +                    System.exit(1);
> > > 
> > > Hmm, is there a less clean way to leave, like abort() vs exit() in C?
> > > 
> > 
> > AIUI abort() and exit() are just as "unclean" as each other.  Maybe I'm
> > misunderstanding what you mean by "clean".  When the program is aborting
> > because of a critical error, it's never really going to be "clean".
> > 
> > Do you want me to change something here?
> 
> abort() raises SIGABRT which will be trapped by the debugger or dump a
> core file if enabled, and doesn't call atexit handlers or destructors.
> 
> I can't find an equivalent for Java so leave it.

Okay thanks.  That's useful to know about abort().


-- 
Paul Bone



More information about the reviews mailing list