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

[Bug]: How to analysis the reason of issue #5743 #3

Open
Bei-Jin-lab opened this issue Oct 16, 2024 · 4 comments
Open

[Bug]: How to analysis the reason of issue #5743 #3

Bei-Jin-lab opened this issue Oct 16, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@Bei-Jin-lab
Copy link
Collaborator

Bei-Jin-lab commented Oct 16, 2024

Summary

The overall Peer Review screen incurs a lot of delays. This is observed even on good internet connections. The peer review screen takes a few seconds in order to load the image to be currently reviewed by the user. The fix for this is important as it would help improve the usability of the feature and would make it accessible to a wider range of users.

User [sivaraam] report: The first part of this enhancement would be to identify which part contributes to the slow loading of the image in the first place.

User [nicolas-raoul] report: Peer review becomes slower and slower. After reviewing a few dozen images, it takes several minutes between each review.

After discuss, User [psh] found the reason: The ReviewActivity runRandomizer() method is getting called many times (in one test, 30+ times), and there are (at worst) a couple of hundred API calls going on until it finds the next image to review. Each API call is taking 150-250ms each.

For now the root cause has been found, We try to fix this bug to reduce delay berween peer reviews, make this process faster.

Steps to reproduce

  1. Recheck if The ReviewActivity runRandomizer() method is getting called many times is the cause of the slow peer reviews.
  2. Check how many times these methods are called.
  3. Reduce the number of times these methods are called
  4. Test the result

Expected behaviour

The number of times these methods are called has been reduced, and peer review will spend only 1s.

Actual behaviour

Device name

Samsung J7

Android version

Android 10

Commons app version

3.1.1

Device logs

No response

Screen-shots

No response

Would you like to work on the issue?

Yes

@Bei-Jin-lab Bei-Jin-lab added the bug Something isn't working label Oct 16, 2024
@Bei-Jin-lab
Copy link
Collaborator Author

First step: Recheck if The ReviewActivity runRandomizer() method is getting called many times is the cause of the slow peer reviews.

In ReviewActivity class, runRandomizer() is called five times;

1. In onCreate():
      if (savedInstanceState != null && savedInstanceState.getParcelable(SAVED_MEDIA) != null) {
          updateImage(savedInstanceState.getParcelable(SAVED_MEDIA));
          setUpMediaDetailOnOrientation();
      } else {
          runRandomizer(); // Called when there's no saved state
      }

   2. In skipImage button's onClick listener:
      binding.skipImage.setOnClickListener(view -> {
          reviewImageFragment = getInstanceOfReviewImageFragment();
          reviewImageFragment.disableButtons();
          runRandomizer();
      });

   3. In updateImage():
      if (media.getUser() != null && media.getUser().equals(AccountUtil.getUserName(getApplicationContext()))) {
          runRandomizer();
          return;
      }

   4. In swipeToNext():
      if (nextPos <= 3) {
          // ... (some code)
      } else {
          runRandomizer();
      }

In the runRandomizer() method, it indeed triggers the API request:
reviewHelper.getRandomMedia() is likely a network request

compositeDisposable.add(reviewHelper.getRandomMedia()
        .subscribeOn(Schedulers.io())
        .observeOn(AndroidSchedulers.mainThread())
        .subscribe(this::checkWhetherFileIsUsedInWikis));

Additionally, the checkWhetherFileIsUsedInWikis method also triggers another API call:

compositeDisposable.add(reviewHelper.checkFileUsage(media.getFilename())
    .subscribeOn(Schedulers.io())
    .observeOn(AndroidSchedulers.mainThread())
    .subscribe(result -> {
        // ... (some code)
    }));

@Bei-Jin-lab
Copy link
Collaborator Author

Check how many times these methods are called in one test

Use counter

public class ReviewActivity extends BaseActivity {
    public static int runRandomizerCallCount = 0;

    @SuppressLint("CheckResult")
    public boolean runRandomizer() {
        runRandomizerCallCount++;

@Bei-Jin-lab
Copy link
Collaborator Author

Through the test, the method has been called over 30 times in one test.

Next step, we goona reduce the number of times these methods are called.

realized by this way:
Make 1 API call to retrieve like 50 pictures to review, then cache the results and use them locally as long as they are not over 1 day old.

@Bei-Jin-lab
Copy link
Collaborator Author

The original issue comment: commons-app#5743

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant