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

Active branch not shown on graph if too many other commits #1293

Open
Hirse opened this issue Mar 12, 2020 · 15 comments
Open

Active branch not shown on graph if too many other commits #1293

Hirse opened this issue Mar 12, 2020 · 15 comments

Comments

@Hirse
Copy link
Contributor

Hirse commented Mar 12, 2020

When opening large repositories, it can easily happen that there are more new commits on other branches than the current initial display limit for graph nodes (25 as far as I can tell).
If the latest commit on the current branch is not within this limit, that branch is not shown at all causing a bit of a broken layout.

Screenshots for better explanation:
Actual:
image

Expected:
image

To reproduce, clone my fork, stay on "master" branch and observe the "empty" branch to take up all the available commit graph nodes.

This seems to be somewhat related to the ongoing effort by @campersau and @jung-kim of optimizing for very large repositories.

@jung-kim
Copy link
Collaborator

So for that one you want to increase maxActiveBranchSearchIteration config value to be a positive number. That number represents number of iterations it may attempt in search for the active branch.

Also you might want to know that each iteration will load up to numberOfNodesPerLoad nodes.

@Hirse
Copy link
Contributor Author

Hirse commented Apr 16, 2020

Yes, that would work, but wouldn't be necessary.

For the issue above, I would just like to show the dashed circle and line indicating the active branch to distinguish it from the "green" branch.
This can be an issue especially in very large repositories, where loading more nodes would be counter-productive.

@jung-kim
Copy link
Collaborator

Problem with that is that we don't know any details of the "active branch" until checkout git node is parsed. By that time we would have all of it's ancestors and other branches so why not just display them rather than the dotted display?

@wmertens
Copy link
Contributor

If we could load all commits into memory, this would be solved. I explain in #1323 how.

@jung-kim
Copy link
Collaborator

Not all repos can fit

@wmertens
Copy link
Contributor

@jung-kim well, I explain how even Linux can fit... The trick is to only use the first 32 bits (the commitish) to reference commits, and then use integer arrays so that most vms can store the info packed.

@jung-kim
Copy link
Collaborator

If we just loaded hashes of all commits into memory, what problem does that solve?

@wmertens
Copy link
Contributor

@jung-kim it would allow knowing the entire shape of the graph synchronously. You'd have a Uint32Array of commitishs, an array of their parent commitshs, and then an overflow map for multi-parent commits.

Then you get all the refs and you can draw the graph of commit dots.
To use timestamps for dot placement, that's another 4MB if you use only relative timestamps (assuming second granularity and a project lifespan of less than 68 years).

Although this is maybe a bit far-fetched.

In any case, the gitlog shouldn't include full data for each commit, it takes a long time to fetch and most of it is unused.

I think that the graph should always include all branches the user wants to see, even if their first commit is beyond the current screenful.

So probably it's better to calculate the shape on the server, and let the client request vertical slices, including the x+y positions of the "previous/next in branch" commits beyond the slice. That way, paging is also simple, you request the next slice, and the graph layout is only calculated once on the server.

GitUp can show really large trees, but it doesn't use timestamps to order the graph, it attaches all commits to the branch head and then a line to the common parent.

@wmertens
Copy link
Contributor

…or: the client decides what to show and how, and gitlog can be requested for only one commit at a time.

So it would work like this:

  • client starts and requests all branches. Gets the commit data for each branch head
  • client decides what branches to show and requests gitlog for each top commit, with or without metadata
  • when you scroll down, it loads more commits per branch by asking gitlog from the last commit onwards

=> no missing commits for drawing possible

As for ordering, perhaps the current HEAD should always "hoist" its commits to the top so that when HEAD is on an old branch you don't have to scroll down to see the first commit.

@jung-kim
Copy link
Collaborator

One of the design principles we stuck with is stateless server design. That way server can be shared.

I don't think what you are proposing is as easy nor beneficial.

Especially for the problem that is simple as, just scroll down.

@wmertens
Copy link
Contributor

It's not just "scroll down" - if you check out a branch that is quite old, you need to scroll down pages and pages before you see the first commit.

I'm trying to solve two problems at once: make gitlog more efficient and fix this issue. I'm thinking that something like the last thing I proposed might work, namely that you get commits for your HEAD and then some more for other branches, but it's not clear to me how many to get.

Suppose your HEAD is 300 commits behind, should you get all the other commits that belong to newer branches that branch off your branch?

@jung-kim
Copy link
Collaborator

It is just "scroll down" though, we already have code that mimics "scroll down" by trying to search for checked out branch up to n nodes.

Feel free to send PR for making it efficient according to what you have described

@wmertens
Copy link
Contributor

wmertens commented Apr 30, 2020

@jung-kim I'm trying to ;) In the nodegit PR I want to replace gitlog with the nodegit equivalent, but

  • it means manually walking the tree in some order
  • it means fetching and sending a lot of data which isn't used immediately, like the filediffs and the message body
  • the current implementation sends from 0 every time, it doesn't page

I didn't look at the UI side yet, could you help me explain things?

  • Is it easy to change the commit display code so it sends a separate request to get the extra data when it needs it?
  • What other commits should ungit have if the code only starts walking the current branch?
    • So suppose HEAD is 300 commits behind the latest branch, what should gitlog send?

@jung-kim
Copy link
Collaborator

Is it easy to change the commit display code so it sends a separate request to get the extra data when it needs it?

No, you can try to take a look at #1277 as that was my attempt loading only the diff git nodes. But it was abandoned for two reasons.

  1. speed benefit it gave for big repos wasn't significant.
  2. it causes slew of bugs, some requires very large redesign. i.e. if new commit comes in, client would need to know to ask nodes that is previous to the known first node. server needs to know the client's states and client needs to be aware of the states. It become very complicated operation. not to mention that diff only operations will be sporadic and controlling all the race conditions that may happen is not easy.

Complexity it brings is just not worth it.

What other commits should ungit have if the code only starts walking the current branch?

Currently we are sending ALL git logs in our own object format, which is viewable if you inspect. As far as what it should send, I thought you had an idea to do that. I just presumed filtered version of what we are sending right now or it filtering will be done at UI only. To be honest, I don't understand your approach to answer this.

@wmertens
Copy link
Contributor

BTW, in #1315 there's already some work in this direction, and it makes sure you see HEAD.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants