Jenkins Platform SIG, 12 Jun 2020

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: 2020 06 12 GSoC Git Plugin Performance Project

Description

Google Summer of Code Jenkins git plugin performance improvement project office hours June 12, 2020. See the meeting notes for details

https://docs.google.com/document/d/1ov4ug9WfbcTYNHL1DBcsxyRKgCi7EnFVIywdiP36CSk/edit#heading=h.ma2iqcagbd5m

A

Has started go ahead.

B

I think you enabled uh screening.

A

Once again, thank you very much. Okay. You can now share your screen. Reshop.

B

Yes, so the the first thing I want to discuss is the agenda yeah. So the first thing is so uh so we have the fix for the jenkins file now, so we're not running the benchmarks everywhere for every pr now. uh One question I had related to it was: should we create a gsog dev branch, uh where I, where I post, where I uh push all of my changes in the get client plugin, not in the master directly and work accordingly? Is that something we want to consider or.

A

Tell tell me what you what you would hope to gain by that is. That is your concern that you might be too disruptive on the master branch or yes, it's an interesting insight.

B

Yes, that's the only concern.

A

And- and I don't think I don't think I I would lobby- let's let's call for the others, but as as a maintainer. I like that the changes are on the master branch, because if you break something I learned very quickly and if it's on a separate branch, the danger is, I won't learn as quickly. I won't be as useful to you, so my my personal preference for right now is: let's keep using the master branch and admit that that that means there's it's not free, but we will get better progress.

A

I think overall, if we stay on the master branch for now, but it's a valid point. If we need to, we could certainly allow a separate branch that you you push to or that you submit pull requests to on on the central, repo, justin or omkar any any opinion.

C

There are you, I think, maybe are you proposing that you would do it on master and another branch, uh no.

B

I just don't want to push my changes directly on master on a separate branch and then and that branch it runs the benchmark so that it's it's it is running verily. Basically, it's just a suggestion. I I really it's not something. I have considered too much. It's just a suggestion. Well,.

C

And I mean sorry guys was gonna say the only thing that I could see an advantage of having it also in another branch would be that if you have changes that you need to make uh in that other branch that you can see it run in ci. But I think I agree with mark too, though, like uh it'd be nice to see it on master as well. If we were to do that, just especially help mr mark.

A

We we could take a variant of that, though we could say, hey we're not even going to run the benchmarks on the master branch, create a gsoc dev branch and only run them on the dev branch.

A

But but for me it feels like hey, I like knowing, and I like, watching the evolution and if you break something on master you'll, get attention much much more rapidly than than if you break something on a separate branch, I'm I'm very attentive to the master branch and particularly now as we're approaching the release of get client 3.3 and get plug-in 4.3. So this is a great time for you to be there so that you're getting lots of lots of attention.

B

Okay. Okay, I understand that so uh the next thing, then, is is something I was thinking about.

B

While uh thinking about this issue, only the branch issue that we currently have my benchmarks for the git client plugin, and uh so I was thinking the scope of our benchmarking project- is to basically understand if we have a faster implementation for certain scenarios and then enhance them in the plug-in, and we also so do we want so uh will we also uh maintain the benchmarks on the cia jenkins infrastructure after we, we use the benchmarks for this project.

B

We want to keep those benchmarks on the uh jenkins infrastructure right, so if we are doing that for git client plug-in what I was thinking.

B

So my concern there is that for for the git client plugin, I I don't see uh rapid changes in that repository, where benchmarking could help that much as compared to the git plugin, where the changes which would be much more, which might where the developers might benefit with a performance uh benchmark, but the concern there another concern there is that we are conducting micro benchmarking here and when we shift to git plugin uh for an example.

B

If I want to benchmark uh the checkout step, that includes a lot of functions and lot of api calls, so it I think uh the definition of micro benchmarking doesn't apply to that to that whole process, but to have a benchmark which would uh which would create a reference for developers when they change the checkout process for some reason and they have a reference and- and that would help them a lot, but but uh conceptually, I'm not sure if the micro benchmarking principles are violated when we are trying to uh record and measure such a such a big step should.

B

Can we do that? Should we do do that, or should we break the checkout step into some pro some stages where we can benchmark it like we are benchmarking the operations we are doing that in the git client plugin, but I'm not sure if the git fetch step is going to be updated, that much so the benchmarks. We have the results, but I I'm not. I don't understand how the benchmarks held that much in that case, so.

A

So, yes, the, I think. Well, there was a recent webinar presented where, by uh by someone from the oh, where were they from it was about a week ago there was a bench: a presentation was made where they they were. They showed using performance data historically to help detect flaws in in something.

A

So I think your idea is valid. However, part of me says this is this is probably the wrong time to scope creep your particular project, the the benchmarking inside git client, still has several different areas that probably justify investigation.

A

After we get fetch understood and after we get it to the point where it's running reliably and that we we've compared its behavior on our pc on system 390, on on windows on linux and we've understood, gathered some historical data so that you could present hey, look, here's the behavior and all these different platforms have these curves that the shape looks like this.

A

That, for me, is already so valuable to to do that kind of thing that I would ignore git plugin for now, just because I think we've got so much to learn and get client.

B

Okay, that seems fair enough. I understand that.

A

Yeah, it's not that it's not that it's not a great idea. I think it's a brilliant idea, but my suspicion is that we've got an awful lot of things that have to happen still for us to say. Yes, we we got this all the way to getting the value out of the benchmarking that we wanted to get.

B

Okay, yeah. I understand that.

C

Maybe that would be a good follow-up uh thing if we end up uh getting far enough along that, you would be able to use the principles you learned in this one and then apply it to the get plug-in, because I think I agree that I agree that uh it's nice to have metrics, especially term, so you can tell like what kind of small things, and I think this is what you're getting at. Also when, when you see a change, that's made a big spike, then you know that's more obvious than a longer term.

C

Oh, I think at some point. In this last year we got a. We had a performance fight.

A

Yeah for for as an example, the place where, where what you're suggesting might be particularly useful, um one of the one of the problems we have in in the project is that it supports a wide, wide range of get versions all the way from the get that shipped with centos 7 1.8. That is now six or seven years old to the most recent get 2.27 and it allegedly supports all of the versions in between.

A

But you can just be confident that 1.8 has a very different behavior performance, wise feature-wise capabilities compared to 2.27, and there are options that we we don't yet use in the brand new versions, because we're allowing ourselves to use the old version to support the old versions.

A

Eventually, I'd love to get to a mode where we say we're going to drop support for get 1.8. We're going to drop support for git versions, up to 2.7, for instance, but in order to do that, we'd have to have a compelling reason which says, because we gained these things and your benchmarks could tell us here- are some of the things we gain so again. Why? I think your idea is good, but I think it's the wrong time for us to approach it. Let's get get the get client, benchmarking, understood and and implemented.

A

B

I I get it okay, so the next thing uh on the agenda is progress on the issue. So first, I think I should I should. uh Should we see the code, should we see the code right now or is it something I I can raise the pr and then you guys can review it? Should we spend time on that.

A

You're, actually I I want you to brag first about the the windows fix that you did so that that, before you show any code, even the test case failures on windows, you've resolved right. So.

B

uh uh You're talking about the assertion, errors right.

A

B

Yeah, so that was resolved by by the thing you gave marked on for the I think in the morning you gave me the solution for it, so I just I just did that. I there was nothing novel in in fixing that those test failures, but I realized that uh maybe we can uh find a better way to assert uh the double that we don't have. The double fetch fetches now.

B

One of the simplest way I could I could think about was to not uh not check if git fetch is there or not just check uh before gate fetch it logs and info that it is fetching the upstream repository.

B

So I think I can count that, and that is agnostic to the platform that would be agnostic to windows or linux or any platform. So that is the simplest thing I I could think of to just count that so I would have just one count after my fix and before my fix it would be two so so I was thinking of implementing that right now I have done what you gave. We're basically matching the pattern and using reject so so I was do.

B

I was thinking of doing that and um so with the so we still have one test case: failure with windows which uh me and mark we're going to explore in debug, and that's it for the test case failures right now and yeah.

A

So so the the one remaining test case failure actually is not windows specific. It's I see it fail also on my linux environment um so, but but that's a relief, because the windows, specific one, had had me completely perplexed. Why why windows? What's so magical about windows, and this the one remaining failure, is both visible on windows and on linux. I'm confident we'll see it anywhere, because there's there's got to be some fundamental, some fundamental thing that we're missing. Just like you found two days ago.

A

You pointed out to us that the extensions were just not being added to the clone command. Yes, but.

B

But what it was um the build is successful. In my look on my local machine mac os system, it's successful, no tests are failing in that much try.

A

Try merging the master branch just just to be sure, try merging the master branch. It may be that you're you're clean before checkout. Well, I I don't yet know what it is, but I definitely see it on my on my linux box.

B

But but it is also succeeding in the uh jenkins infrastructure linux instances, both linux instances. If you, if you check, are you sure? Oh that's interesting, that's right I'll, show you oh and master uh yeah I yeah this is the branch.

C

Maybe mac is special now no, I'm just kidding.

A

Right: it's it's especially a berkeley linux! That's right! That's about how special it is! It's a special berkeley, linux for sure interesting, oh and you're, right, wow! Okay! Why do I see it consistently on my my linux computer? I just ran it minutes before this meeting and saw the failure.

B

No failure interest- it's not failing in my on my machine and in those linux instances. So first I initially, I thought it's something. Let it do windows I could not understand, because it's versions.

C

uh Is this one using the uh the system get or the jig it? uh It's.

A

Let's see the failure is using, it's get us get a cm test, so it's using cli kit right.

B

All the tests, don't they use jget.

A

B

A

I don't know how it could, since if it were using j get we wouldn't. We wouldn't be able to assert on the double fetch, because you were asserting for specifically the command git space. Fetch and jet does not echo get fetch, but so I'm pretty sure that choosing cli kit. But it's a valid question justin and a really good one for me to double check to be sure that it's using, let's see, is get scm test, the one that is parameterized.

C

One thought that I have is that uh sometimes, if you rely on the text of a of an output and that the thing that puts out that output may change over time, and then you get fun little test things. And then I'm especially kind of like concerned by what mark had said in terms of the spread of git client versions, which totally makes sense that you would support that wide of a spread.

C

um They've changed the text in uh in the git fetch.

A

Output yeah, in this case the the fetch that he's the the text he's asserting on it's a good point. It's a very good point. The text that he's asserting on is actually text generated by the by the get client plug-in itself, not by a command line kit. So it's a diagonal. No, no! It's a very good point. It's a diagnostic output provided by the the plug-in that happens to be a copy of the arguments it passes to get fetch cool, much safer, well, sort of yeah.

A

It has exactly you you, you hit it exactly right that it has yeah. So I rishabh, I definitely see the same failure.

A

Let's see what my status is yeah, so I did do a merge from the master branch, but all that changes is brings in some documentation changes compared to what you had. So I I don't think that alters your should alter the behavior at all. So this will be a fun investigation. Thank you for for giving us an opportunity to to look at something um together. That's gonna be interesting.

A

I I don't think we should keep you awake significantly longer it's getting late in your day, right, we're we're probably at it's 8 20 right now: 8 20 p.m. Okay, so it's rishabh and I were justin rashov and I were talking 10 hours ago and he's not slept since then. I had a good night's sleep so.

C

B

Okay, so um so this is what the test case failures after this. um The next thing is that I have to create test cases for the for all of the addition of extensions I've done, and uh so I was I was seeing that we don't have any tests for uh pruning stale branches, so there is no test for that. So I would sadly add a before and after test for for that feature and then um for pruning tags, that, for that also I'm going to add automated test cases and uh for the clean before checkout.

B

Do we need to add. uh Do we need to add explicit, because I think there was an excess existing test case, which uh pointed out uh to me that I am I'm fig, I'm I'm missing something. So do we need uh two more tests which explicitly one more test or maybe two which explicitly tell us that okay, this fix is doing this no.

A

No not for not for me. I think I think yes, I I'm embarrassed that we got lucky, but I'm glad that we got lucky now you're going to reduce our odds of being lucky in the future, by adding more tests to reduce the lucky, the reliance on luck and instead rely on skill of tests.

B

Okay, okay, so, and also.

C

B

Yeah yeah, yes,.

C

Yeah I'd say I wouldn't worry about retrofitting tests. Well, unless it's something that we're gonna change.

A

To you yeah, I I hang my head in shame that there were no tests on prune branches, but I think that's older functionality. I'm really embarrassed that it didn't catch it on prune, stale tags, because that was recently added that hasn't even been released. Yet that'll first come out in 4.3, so that's brand new functionality that I missed getting a test into it. So thank you for catching. It.

B

Okay and after that, I I need to create a benchmark for for to point out the performance difference between the double and the single fetch. I actually have a benchmark, but it's not working right now. It worked before during the proposal. I was creating it's not working right now, so I'm going to fix that, and uh so I have some so I after the fakes I did did a few, uh so I I I put system.nanotime in the retrieve changes stage of the checkout. I want to show using the code where I've done it.

B

uh Yes, so this is the stage where we basically call git fetch twice. So I I just did this to check. uh Once I put my fix, what is the time difference? What is happening so the result was a difference of 20 seconds. For uh so I ran it. So I uh I built it for jenkins, I o repository, which is around 40 mb right mark. uh Yes,.

A

B

So, uh for for that repository, the difference is around 20 seconds after uh after fixing it which, which is okay right.

A

That's substantial: that's! I expected it to be near zero because I think of fetch to get nothing should be pretty cheap. So that's that's quite impressive. To be able to save 20 seconds, lots of people should be very happy. Yeah.

B

But uh code is it: is it possible that this is something related to my machine? It could the time difference or the time it could be corrupted, because I am using system.nanotime and I am uh performing it on my local machine. So we would have this time difference or.

A

So so the answer to the could could a benchmark be skewed by something is always yes, but in this case the difference seems large enough that it's unlikely any skewing inside your system is really impacting that number. So so, yes could the benchmark be skewed. Absolutely, uh I suspect, if we run these same kinds of tests, multiple times we'll get repeated or in multiple locations, those kind of things we'll get repeated evidence that you've got exactly the what you're observing is exactly correct.

A

Taking out that second fetch has a a very distinct detectable improvement performance.

B

Okay- and I also uh tried profiling, the jenkins for by adding a custom build of my plugin inside the plugin directory, but uh I'm not not able to get any difference in the thread dump here. It's it's it's the same, I'm not sure. If, uh if jenkins, I I saw the bit, I saw the logs when the jenkins instance was starting up, so it was using the current uh gen, the plug-in gate plug-in I provided to it, but but I didn't change the git client plugin code.

B

It couldn't be because of that because, um because my fix is also using some functionalities of the git client plugin, uh which which, which is a which I have updated, because it's not available in the uh in this jenkins repo right uh from where we're downloading the plugins.

A

Well, so so how.

B

A

Correct, how did you? How did you launch jenkins here? Did you use maven, hpi, colon run or some other technique to launch the war file? I I use java hyphen jar jenkins. Oh okay, good! Well! So then, having done that, you'll find. um If you look in the jenkin in the man in the system, information for your jenkins. While it's running it will show you what it's jenkins home directory.

A

Is you find that home directory copy your getclientplugin.hpi file into that that directory's plugins directory as a jpi and restart jenkins, and it will now use your new get client, so you, you, then have the convenience of you can put both a new git client and a new git plug-in into that same location, and it will use them both.

B

Okay, I did that for git cloud plug-in, but I did not do it for git client so I'll do that.

A

So do the same for git client see. I would expect that if you didn't do it for get client, you wouldn't get the remove of redundant fetches. Would you because the remove redundant fetches is only implemented in git client plugin in the git plugin?

A

Oh okay, great! My mistake, that's great! So it's that's! The implementation of that is actually in get plug-in, not get client.

B

Gate client, yes,.

A

Great okay, thanks excuse, my forgetting.

B

That's okay, so uh so here you can see that we have two gate fetches. Actually we have multiple gateways. I don't understand so I I think I need to understand the the thread dumps more because I'm seeing two fetches there and then I'm also seeing get fetch after this just one minute.

B

B

I'm not able to find it right now. I saw.

B

So so I have two uh recordings. The this one is from uh before the fix. So here you can see two more so two fetches there and then we go up where it is actually using fetch to yeah two more fetches. So this is something I I don't understand. Is it? Is it because it is it paralyzing?

B

Is it because of multi-threading, or why would this happen that I am seeing three four five fetches are.

C

There any some modules.

B

A

So is, is the is the job you're testing a pipeline job or just a freestyle job.

B

Please take a job: okay,.

A

All right so then, then you you. If if it were had been a pipeline job, it would have potentially been loading pipeline, shared libraries, so that might have explained multiple fetches. In this case, I I don't have a good explanation for why it would do four fetches, instead of one or two, depending on whether you had redundant fetches removed or not. So I I don't know, that's a good question.

C

Okay, so maybe I know what the differences are in the in the commands.

A

Now, well so I guess you'd, I assume that you do not have um wipe workspace extension added and you don't have calculate change log and you don't have any of the other that you've captured number of extensions, very simple.

B

Yes, it's no extension, it's just a checkout great.

A

So certainly worth investigating, I I don't directly know oh yeah. This is why you would see that I have I'll have to look it's it's a good question.

B

Okay, I'm going to investigate more yes, so uh so I think the tasks I now have are the first one is to raise the prs with the automated tests.

B

The second one is to create the benchmark so that we have a good visual report of how this this fix has improved the performance and um and the third one would be to find out it's it's. The same I had on wednesday is to find out a way to implement the opt-in feature. We want to include for the performance enhancement as an extension which we were talking about so yeah, so I'm going to do that doing that during this weekend, hopefully yeah.

A

I wonder: okay, this is where justin and omkar get to chime in a part of me is tempted to say hey. Should we implement it as just a single global switch, which is go back to the old way of doing things or keep doing things the new way? So so there is, there are global switches you can set on the get plug-in that are available through the manage jenkins page, and there are relatively few of them right now.

A

Configuration username um whether or not it should create new users and one other, but but is it get to.

B

A

B

Git to selection of git to git or jk.

A

No it's this is this is this, is this is at the here? Maybe maybe what we can do is, if you don't mind I'll, just share with you, so that you can see it. So you know where to look.

B

A

Because I'm prone to say you know what maybe this is so valuable that we should make it the new default and only allow people to go back to the old default. The old way of doing things if they find some catastrophic problem, because making people turn it on will slow the adoption of it being switched on the. But if we don't allow them to turn it off, we may break them catastrophically.

C

And they will be upset.

A

Right exactly so, if you want to stop sharing briefly, I can show you where.

C

A

C

uh I saw that those had stand here, std, air and std out. I wonder if those are binding to two different uh streams in java. Maybe that's why that's showing up twice.

A

Oh, oh yes, yes, you're right, look at what it says there standard out copy or standard error copier, and they are in fact two different file. Descriptors, very good justin! You win bonus points. I think you're right, that's great! Okay! You might want to double check that because that's.

B

A

About 8 a.m, and it has to read two different file descriptors right because they get command line, get really writes some things to standard out and some things to standard error. We bind both of them, so we get everything very good. Nice.

C

And you should be good.

B

Yes, so I would have four fetch statements there: okay,.

A

Understood: okay, so, okay, here's my share screen this one! Okay! So what you see here you should see. Do you see my jenkins instance.

B

A

Okay, good, so if we do manage jenkins system configure system- and this particular system has several thousand jobs on it and a bunch of global configuration, so it'll take a while to load. So while it's loading, what will appear if we search for the string get space plug-in on this page?

A

Get space plug-in you'll, see this section here, this block of global configuration settings, so I can set the who, who should? What? Should the username be for anything that is committed, and this is one example- show the entire commit summary and changes of a place where a new behavior was desired and we allowed people if they didn't like it to check this thing off.

A

So it was so, and this is so. If you look for that string in the in the uh in the source code, you'll see a pattern.

A

That's used to implement that kind of thing, and it may be that at least my initial take having having seen the work you're doing and the results you're getting that we think removing redundant fetch should be completely transparent to the user and if it truly is transparent to the user, then there's no reason not to just have a switch here which says um enable redundant fetch and it defaults to false right.

A

It's it'd be the slow me down, please mode for compatibility.

B

Okay yeah, we could do that. Yes, although I don't understand, why would people choose it if, if you're not breaking any use case.

A

Well, well, the notion is we, we broke their use case and they don't have time to wait for a fix.

C

Yeah, I guess one of the things that I I don't know. I don't know if it would be nice to have it both in admin and and in user space, just in case it's a particular repository, that's breaking and all the other ones. They want the performance stuff, but uh then we're starting to get into gold plating and stuff like that, but justin. That's.

A

Omkar, you are welcome to drop. You don't need to remain on. I think we finished rishabh anything else. We need to do.

B

No, no, I think it's I've discussed what I want to do for today.

A

Excellent, we will meet again wednesday, hope that we've got solutions and we'll talk about the next steps. Then, okay, thanks everybody bye, guys, okay,.