Performance Issue in getJustInTimeBinding Method

[vinay-sangwan]

In getJustInTimeBinding(Key key, Errors errors, JitLimitation jitType) method we are taking lock even in cases where we have already cached result in jitBindingData. Which is affecting our application performance.

Below screenshot of code block where this issue is present.

Suggested changes : Can we return BindingImpl if its already present in our cache instead of taking lock , this will remove unnecessary contentions among threads ?

[cheenar]

The method exposing this function, getBindingOrThrow is defined as

<T> BindingImpl<T> getBindingOrThrow(Key<T> key, Errors errors, JitLimitation jitType)
      throws ErrorsException {
  // Check explicit bindings, i.e. bindings created by modules.
  BindingImpl<T> binding = bindingData.getExplicitBinding(key);
  if (binding != null) {
    return binding;
  }

  // Look for an on-demand binding.
  return getJustInTimeBinding(key, errors, jitType);
}

If possible, you could side-step this by just having your bindings explicitly in your module. Alternatively, I wonder if your proposal has any meaningful performance impact -- the synchronized block allows the cache to synchronously checked for a value before attempting to write to it. If you duplicated the read block, i.e. you just check the cache before the synchronized and then once again in the block, you do not lose the synchronous read/write (safe) but in exchange for the latency to recurse up.

private <T> Optional<BindingImpl<T>> findJitBindingFromCache(Key<T> key, Errors errors, JitLimitation jitType) throws ErrorsException {
  boolean jitOverride = isProvider(key) || isTypeLiteral(key) || isMembersInjector(key);
  for (InjectorImpl injector = this; injector != null; injector = injector.parent) {
    @SuppressWarnings("unchecked") // we only store bindings that match their key
    BindingImpl<T> binding = (BindingImpl<T>) injector.jitBindingData.getJitBinding(key);

    if (binding != null) {
      // If we found a JIT binding and we don't allow them,
      // fail.  (But allow bindings created through TypeConverters.)
      if (options.jitDisabled
          && jitType == JitLimitation.NO_JIT
          && !jitOverride
          && !(binding instanceof ConvertedConstantBindingImpl)) {
        throw errors.jitDisabled(key).toException();
      } else {
        return Optional.of(binding);
      }
    }
  }
  return Optional.empty();
}

/**
 * Returns a just-in-time binding for {@code key}, creating it if necessary.
 *
 * @throws ErrorsException if the binding could not be created.
 */
private <T> BindingImpl<T> getJustInTimeBinding(Key<T> key, Errors errors, JitLimitation jitType)
    throws ErrorsException {
  final Optional<BindingImpl<T>> cachedBinding = findJitBindingFromCache(key, errors, jitType);
  if (cachedBinding.isPresent()) {
    return cachedBinding.get();
  }

  synchronized (jitBindingData.lock()) {
    // first try to find a JIT binding that we've already created
    final Optional<BindingImpl<T>> synchronizedBinding = findJitBindingFromCache(key, errors, jitType);
    if (synchronizedBinding.isPresent()) {
      return synchronizedBinding.get();
    }