Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [e4-dev] Context recording w/ RunAndTrack and ContextObjectSupplier

Hi Oleg,

Thanks for your reply.

I agree the behavior you describe is perfectly normal. My problem is a
bit different.

The word "track" seems to be applied to two concepts here:
(1) should injectors track changes so that it can inject the updated
instance later
(2) should the context accesses be recorded (trackAccess() method and
dependencies stuff) so that the context only calls
RAT#changed(IEclipseContext) on appropriate events

(1) needs (2) for context injection (@Inject as in your moo() method),
but (2) is useful on its own when we runAndTrack a given context.

My example is as follow (simplified pseudo-code from my mind, I'm away
from work):

------------------------------8<------------------------------
private final class CmdRunAndTrack extends RunAndTrack {
  @Override
  public boolean changed(IEclipseContext context) {

        ECommandService commandService = context.get(ECommandService.class);
        EHandlerService handlerService = context.get(EHandlerService.class);

        ParameterizedCommand command =
          commandService.createCommand(getCommandId(),
            getCanExecuteParameters());
        boolean enabled = handlerService.canExecute(command);

        // use enabled value for stuff

  }

}

and later on

context.runAndTrack(new CmdRunAndTrack());

------------------------------8<------------------------------

Now, the whole interest of that code is that the @CanExecute parameters
of the handler EHandlerService is wired to are going to be tracked by
that CmdRunAndTrack instance. It does work in last July's release.

The problem is that EHandlerService#canExecute calls the MethodInjector
in such a way that _context access recording_, and not just tracking, is
disabled. I don't want @Inject tracking here, I'm not expecting it from
@CanExecute.

The problem is the fact that @CanExecute prevents the enclosing
RunAndTrack to know about the context lookups that it needs to track Cf
the code I quoted in my first message, ContextObjectSupplier (wider scope):

                if (requestor != null && track) { // only track if requested
                        if (initial) {
                                RunAndTrack trackable = new
ContextInjectionListener(context, actualArgs, keys, active, requestor,
group);
                                context.runAndTrack(trackable);
                        } else { // we do track if this is done inside a
computation, but don't create another runnable
                                fillArgs(actualArgs, keys, active);
                        }
                } else { // This is where @CanExecute calls end up
			if (descriptors.length > 0) {
				pauseRecording();
				try {
					fillArgs(actualArgs, keys, active);
				} finally {
					resumeRecoding();
				}
			}

                }


As you see, the first conditions tests for shouldTrack cases: Here you
do start a new RunAndTrack for those parameters to deal with @Inject,
and you don't in the second case (where there is no tracking required).
The problem is that in that second case, the pauseRecording() call
prevents an enclosing RunAndTrack, such as my CmdRunAndTrack class, to
work properly. My fix is to remove this pauseRecording() call (and the
corresponding resumeRecorded()) so that the parameters of my handler's
@CanExecute method can be tracked by my outer RAT.

We can continue this discussion over at Bugzilla if you feel this is a
bug. I started here to know if you had any idea on how I could work
around this problem without patching core.contexts or if this was indeed
a bug (and to discuss that use case in general).

About your changes in #357268, I'm not sure how they will impact me
(I'll have to test), but as long as context.runAndTrack() keeps the same
contract my code should be fine!

Thanks again,

Simon


Oleg Besedin wrote on 15/09/2011 19:34:
> Hi Simon,
> The code path you are quoting in theContextObjectSupplieris for requests
> that we do not track (see the "if" above it.), so probably the actual
> issue is somewhere else.
> 
> The change done in 4.1 was to stop recording "internal" access in the
> methods that we are calling through injection. For instance:
> 
> @Inject
> moo(arg1, arg2, context) {
>         context.get(arg3); // <- this access we do not want to track
> }
> 
> The arg1 and arg2 will be tracked, but not arg3. That change was
> intentional. (If #moo() is called via RunAndTrack, all arg1, arg2, and
> arg3 will be tracked.)
> 
> I am not sure if this is what you see. If it is not, be sure to open a
> bug with code that shows the issue.
> 
> (Note that dependency handling will be updated in a bit, see patch on
> the bug _https://bugs.eclipse.org/bugs/show_bug.cgi?id=357268_. I
> switched to keeping a stack of "recording states" in there, not sure if
> it is going to affect your code.)
> 
> Sincerely,
> Oleg Besedin
> 
> 
> 
> 
> From: 	Simon Chemouil <eclipse@xxxxxxxxxxxxxx>
> To: 	E4 Project developer mailing list <e4-dev@xxxxxxxxxxx>
> Date: 	09/15/2011 11:04 AM
> Subject: 	[e4-dev] Context recording w/ RunAndTrack and
> ContextObjectSupplier
> Sent by: 	e4-dev-bounces@xxxxxxxxxxx
> 
> 
> ------------------------------------------------------------------------
> 
> 
> 
> Hi there!
> 
> I think this is Oleg's area of expertise... (I hope to catch his eye :)
> 
> I'm currently moving our application to "4.1" (or org.eclipse.e4 0.10.0
> I think), using the Indigo release. It's going surprisingly well, but as
> expected I've spotted a few bugs/problems that either:
> (1) I can work around in client code
> (2) Are clearly bugs that I have reported/will report and provide
> patches for
> (3) I have no idea why this was changed :)
> 
> I've found a problem of the (3rd) kind today regarding Context recording
> (for RunAndTrack) and ContextObjectSupplier.
> 
> Here's our scenario:
> - We have developped a util class that binds a widget (such as Buttons,
> ToolItems, MenuItems, ...) to a commandId in a given eclipseContext
> - This class uses a runAndTrack Runnable that calls
> handlerService.canExecute(boundCommand);
> 
> Since it's tracked in a RAT call, our runnable will be called only on
> context computations that were called the first time, and in my case I'm
> particularly interested in the injected parameters of my handler's
> @CanExecute
> 
> Those parameters are resolved by a ContextObjectSupplier instance.
> 
> Problem is, since 4.1, ContextObjectSupplier does _not_ "record/track"
> access when doing a simple "get" (it still does for tracking injection
> such as @Inject calls).
> 
> Code in ContextObjectSupplier:
> 
> line 162:
>                                                   if (descriptors.length
>> 0) {
>                                                                  
>  pauseRecording();
>                                                                    try {
>                                                                        
>             fillArgs(actualArgs, keys, active);
>                                                                    }
> finally {
>                                                                        
>             resumeRecoding();
>                                                                    }
>                                                   }
> 
> If I remove pauseRecording(); and resumeRecording(), my util class works
> just fine, the context access is recorded and the context RAT knows that
> it must call my runnable when any of these will change....
> 
> 
> Why was this changed? Obviously there is a performance factor, but I
> think my usage was valid. An ugly workaround would be to force the
> context resolution of the @CanExecute parameters from my class, but that
> would involve to introspect it twice more.
> 
> Another possibility is to give client access to whether our
> EHandlerService#canExecute() will track context access...?
> 
> In any case, for now the solution I will likely go for is to use/provide
> the patched core.contexts bundle but I try to stay on the E4 mainline so
> discussion / reflexion is more than welcome.
> 
> Cheers,
> 
> -- 
> Simon
> _______________________________________________
> e4-dev mailing list
> e4-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/e4-dev
> 
> 
> 
> 
> _______________________________________________
> e4-dev mailing list
> e4-dev@xxxxxxxxxxx
> https://dev.eclipse.org/mailman/listinfo/e4-dev



Back to the top