Page unresponsive when loading report of many repos with huge date gap #542 #596

emer7 · 2019-03-11T03:29:51Z

RepoSense is unresponsive if we load many repos
with huge date gap, making it nearly unusable.

This is caused by the report generated having a lot of
empty commits per day, in which the frontend needs to
sift through to display the report.

Let's fix this issue by removing all those empty commits,
including only commits that are necessary.

…mits

emer7 · 2019-03-11T10:19:01Z

@reposense/stage1-reviewers Ready for review

fzdy1914

It seems that you have added many startDate to the functions. Why is this necessary?

emer7 · 2019-03-11T15:58:58Z

It seems that you have added many startDate to the functions. Why is this necessary?

Which one? Is it in the .java file?

If yes, it is because now we don't store empty commits per day but instead we only store meaningful commits. We need start date to count the variance.

emer7 · 2019-03-14T01:42:47Z

@reposense/stage1-reviewers if no one will review, I will proceed with requesting review from stage2-reviewers. I'll request for review from stage2 by 4pm (end of CS3282 class)

yamidark

Some comments for now

src/main/java/reposense/commits/CommitResultAggregator.java

yamidark · 2019-03-14T07:32:08Z

src/systemtest/resources/noDateRange/expected/reposense_testrepo-Charlie_master/commits.json

+    "charlesgoh": 87747.24,
+    "jeffreygohkw": 72073.97,
+    "Esilocke": 419051.7,
+    "wangyiming1019": 64918.33


Why is the variance changing though?

@yamidark
I have investigated it.
I think it is float operation

Consider this arguments
--repo https://github.com/reposense/RepoSense.git --since 01/03/2019
The variance for the old implementation will be 2149.7754
The variance for the new implementation will be 2149.7756

I think the change is caused by the different order of evaluation of totalContribution
For the old implementation, the order will be as follow:
0 0 0 0 0 0 0 0 174 0 0 0 0
While for the new implementation, the order will be as follow:
0 174 0 0 0 0 0 0 0 0 0 0 0

Fixed, I change implementation

eugenepeh

Is there any particular reason that only selective dates were removed?

frontend/src/static/js/api.js

emer7 · 2019-03-16T05:32:07Z

Is there any particular reason that only selective dates were removed?

Those empty contributions that is not removed comes from
List<CommitResult> commitResults = CommitInfoAnalyzer.analyzeCommits(commitInfos, config);

commitResults contains empty contributions.

I do not investigate why so. I only uses what I get from commitResults

eugenepeh · 2019-03-16T10:03:59Z

Those empty contributions that is not removed comes from
List commitResults = CommitInfoAnalyzer.analyzeCommits(commitInfos, config);

commitResults contains empty contributions.

I do not investigate why so. I only uses what I get from commitResults

From what I perceived, it seems to be that an empty contribution will be added to all authors for every commit. The commit should only be attributed to its author and not all the contributors in the repo. Can you look into it and fix this?

emer7 · 2019-03-18T15:52:28Z

Those empty contributions that is not removed comes from
List commitResults = CommitInfoAnalyzer.analyzeCommits(commitInfos, config);
commitResults contains empty contributions.
I do not investigate why so. I only uses what I get from commitResults

From what I perceived, it seems to be that an empty contribution will be added to all authors for every commit. The commit should only be attributed to its author and not all the contributors in the repo. Can you look into it and fix this?

Okay I think better to fix this at separate PR

eugenepeh

minor nit

eugenepeh · 2019-03-21T12:35:37Z

src/main/java/reposense/commits/CommitResultAggregator.java

@@ -19,6 +19,9 @@
 */
 public class CommitResultAggregator {

+    private static final int DAYS_IN_MS = 24 * 60 * 60 * 1000;
+    private static Date lastDate = null;


What is the assumption that the lastDate can be a global variable here?
Perhaps make it a returning value in the methods through pointer etc.

src/main/java/reposense/commits/CommitResultAggregator.java

emer7 · 2019-03-24T03:22:25Z

@reposense/stage2-reviewers ready for review

eugenepeh

minor nits, good to go from me after the fix

src/main/java/reposense/commits/CommitResultAggregator.java

emer7 · 2019-03-24T14:47:05Z

@reposense/stage2-reviewers fixed last nit spotted by Eugene

codecov · 2019-03-26T15:12:59Z

Codecov Report

Merging #596 into master will increase coverage by 0.2%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##             master     #596     +/-   ##
===========================================
+ Coverage     80.64%   80.84%   +0.2%     
- Complexity      537      541      +4     
===========================================
  Files            68       68             
  Lines          1767     1770      +3     
  Branches        185      189      +4     
===========================================
+ Hits           1425     1431      +6     
+ Misses          247      245      -2     
+ Partials         95       94      -1

Impacted Files	Coverage Δ	Complexity Δ
...java/reposense/commits/CommitResultAggregator.java	`96.87% <100%> (+3.43%)`	`19 <1> (+3)`	⬆️
...posense/commits/model/AuthorDailyContribution.java	`60% <0%> (+5%)`	`4% <0%> (+1%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ccea6fd...b6b26aa. Read the comment docs.

yamidark

Some nits, rest LGTM

src/main/java/reposense/commits/CommitResultAggregator.java

frontend/src/static/js/v_summary.js

emer7 added 5 commits March 10, 2019 01:56

Slice and commit position by time to prepare for removal of empty com…

7369f4d

…mits

Reduce number of empty commits in commits.json

6405793

Use sinceDate from summary.json

868177c

Fix spacing for commit and week view

d9a015f

Fix variance calculation

81435d0

emer7 changed the title ~~[WI] Page unresponsive when loading report of many repos with huge date gap #542~~ [WIP] Page unresponsive when loading report of many repos with huge date gap #542 Mar 11, 2019

emer7 added 3 commits March 11, 2019 17:56

Edit systemTest expected commits.json

4b9d8f0

Fix eslint

9bc8393

Fix sinceDate if sinceDate is not supplied

4bda5d8

emer7 changed the title ~~[WIP] Page unresponsive when loading report of many repos with huge date gap #542~~ Page unresponsive when loading report of many repos with huge date gap #542 Mar 11, 2019

emer7 requested a review from a team March 11, 2019 10:15

fzdy1914 reviewed Mar 11, 2019

View reviewed changes

emer7 mentioned this pull request Mar 11, 2019

RampChart: show up to report generation date, not last commit date #518 #539

Merged

Merge branch 'master' into 542-huge-date-gap

9b735bf

yamidark requested a review from a team March 12, 2019 13:33

yamidark added the s.ToReview label Mar 12, 2019

fzdy1914 approved these changes Mar 14, 2019

View reviewed changes

emer7 requested a review from a team March 14, 2019 06:14

Merge branch 'master' into 542-huge-date-gap

fa26ea6

yamidark reviewed Mar 14, 2019

View reviewed changes

eugenepeh reviewed Mar 15, 2019

View reviewed changes

frontend/src/static/js/api.js Show resolved Hide resolved

yamidark added s.Ongoing and removed s.ToReview labels Mar 17, 2019

emer7 mentioned this pull request Mar 18, 2019

summary.json: move sinceDate and untilDate become direct element #607

Closed

emer7 requested a review from a team March 21, 2019 04:43

eugenepeh reviewed Mar 21, 2019

View reviewed changes

emer7 added 5 commits March 22, 2019 01:32

Merge branch 'master' into 542-huge-date-gap

0b5f088

Fix formatting

ef021c7

Refrain from using lastDate as global variable

5c7b86b

Fix style

bae7370

Fix empty commitResults

6e53b45

emer7 force-pushed the 542-huge-date-gap branch from 20a93c2 to 6e53b45 Compare March 23, 2019 17:38

Merge branch 'master' into 542-huge-date-gap

fb41321

emer7 requested a review from a team March 24, 2019 03:22

eugenepeh approved these changes Mar 24, 2019

View reviewed changes

src/main/java/reposense/commits/CommitResultAggregator.java Outdated Show resolved Hide resolved

Fix style

cf53c8f

emer7 requested a review from a team March 24, 2019 14:37

Merge branch 'master' into 542-huge-date-gap

74429e4

yamidark approved these changes Mar 27, 2019

View reviewed changes

src/main/java/reposense/commits/CommitResultAggregator.java Outdated Show resolved Hide resolved

src/main/java/reposense/commits/CommitResultAggregator.java Outdated Show resolved Hide resolved

src/main/java/reposense/commits/CommitResultAggregator.java Outdated Show resolved Hide resolved

Fix styling

badffc7

ongspxm reviewed Mar 28, 2019

View reviewed changes

emer7 added 3 commits March 28, 2019 14:58

Make code more implicit

eb52961

Add comments

571ad13

Improve readability

801c20f

ongspxm approved these changes Mar 28, 2019

View reviewed changes

Merge branch 'master' into 542-huge-date-gap

b6b26aa

eugenepeh added s.ToMerge and removed s.ToReview labels Mar 28, 2019

eugenepeh added 2 commits March 28, 2019 23:34

Merge branch 'master' into 542-huge-date-gap

67a908b

Merge branch 'master' into 542-huge-date-gap

65ebe94

eugenepeh merged commit f278e75 into reposense:master Apr 1, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Page unresponsive when loading report of many repos with huge date gap #542 #596

Page unresponsive when loading report of many repos with huge date gap #542 #596

emer7 commented Mar 11, 2019 •

edited

emer7 commented Mar 11, 2019

fzdy1914 left a comment

emer7 commented Mar 11, 2019

emer7 commented Mar 14, 2019 •

edited

yamidark left a comment

yamidark Mar 14, 2019

emer7 Mar 18, 2019

emer7 Mar 18, 2019

eugenepeh left a comment

emer7 commented Mar 16, 2019 •

edited

eugenepeh commented Mar 16, 2019 •

edited

emer7 commented Mar 18, 2019

eugenepeh left a comment

eugenepeh Mar 21, 2019

emer7 commented Mar 24, 2019

eugenepeh left a comment

emer7 commented Mar 24, 2019

codecov bot commented Mar 26, 2019 •

edited

yamidark left a comment

Page unresponsive when loading report of many repos with huge date gap #542 #596

Page unresponsive when loading report of many repos with huge date gap #542 #596

Conversation

emer7 commented Mar 11, 2019 • edited

emer7 commented Mar 11, 2019

fzdy1914 left a comment

Choose a reason for hiding this comment

emer7 commented Mar 11, 2019

emer7 commented Mar 14, 2019 • edited

yamidark left a comment

Choose a reason for hiding this comment

yamidark Mar 14, 2019

Choose a reason for hiding this comment

emer7 Mar 18, 2019

Choose a reason for hiding this comment

emer7 Mar 18, 2019

Choose a reason for hiding this comment

eugenepeh left a comment

Choose a reason for hiding this comment

emer7 commented Mar 16, 2019 • edited

eugenepeh commented Mar 16, 2019 • edited

emer7 commented Mar 18, 2019

eugenepeh left a comment

Choose a reason for hiding this comment

eugenepeh Mar 21, 2019

Choose a reason for hiding this comment

emer7 commented Mar 24, 2019

eugenepeh left a comment

Choose a reason for hiding this comment

emer7 commented Mar 24, 2019

codecov bot commented Mar 26, 2019 • edited

Codecov Report

yamidark left a comment

Choose a reason for hiding this comment

emer7 commented Mar 11, 2019 •

edited

emer7 commented Mar 14, 2019 •

edited

emer7 commented Mar 16, 2019 •

edited

eugenepeh commented Mar 16, 2019 •

edited

codecov bot commented Mar 26, 2019 •

edited