{"thread":{"id":"50890","subject":"[GSoC][RFC] Proposal: Make pack access code thread-safe","startedAt":"2019-04-07T20:48:43Z","lastAt":"2019-04-09T05:54:55Z","messageCount":13,"participants":["Matheus Tavares Bernardino","Christian Couder","Duy Nguyen","Philip Oakley","Torsten Bögershausen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"373368","messageId":"CAHd-oW7KMrDJ-cyzk63oqW9-QVpag6fKnDp+Mo5bWxg1KfzY3g@mail.gmail.com","threadId":"50890","inReplyTo":null,"subject":"[GSoC][RFC] Proposal: Make pack access code thread-safe","fromName":"Matheus Tavares Bernardino","fromEmail":"matheus.bernardino@usp.br","sentAt":"2019-04-07T20:48:30Z","receivedAt":"2019-04-07T20:48:43Z","isPatch":false,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"Hi, everyone\n\nThis is my proposal for GSoC with the subject \"Make pack access code\nthread-safe\". I'm late in schedule but I would like to ask for your\ncomments on it. Any feedback will be highly appreciated.\n\nThe \"rendered\" version can be seen here:\nhttps://docs.google.com/document/d/1QXT3iiI5zjwusplcZNf6IbYc04-9diziVKdOGkTHeIU/edit?usp=sharing\n\nI kindly ask you to read the text at the google docs link, because in\nthe conversion to plain text I noticed it discards some information :(\nBut for those who prefer to comment by email, here it is:\n\nThanks,\nMatheus Tavares\n\n=======================================\n\nMaking pack access code thread-safe\nApril, 2019\n\n#Contact Info\n\nName Matheus Tavares Bernardino\nTimezone GMT-3\nEmail matheus.bernardino@usp.br\nIRC Nick matheustavares on #git-devel\nTelefone [...]\nPostal address [...]\nGithub https://github.com/MatheusBernardino/\nGitlab https://gitlab.com/MatheusTavares\n\n# About me\n\nI’m a senior student at the University of São Paulo (USP), attending the\nBachelor’s degree in Computer Science course. Currently, I’m at the end\nof a one year undergraduate research in High-Performance Computing. The\ngoal of this project was to accelerate astrophysical software for black\nhole studies using GPUs. Also, I’m working as a teaching assistant on\nIME-USP’s Concurrent and Parallel Programming course, giving lectures\nand developing/grading programming assignments. Besides parallel and\nhigh-performance computing I’m very passionate about software\ndevelopment in general, but especially low-level coding, and FLOSS.\n\n# About me and FLOSS\n\n## Linux Kernel\n\nLast year, I started contributing to the Linux Kernel in the IIO\nsubsystem, together with a group of colleagues. I worked with another\nstudent, to move the ad2s90 module out of staging area to Kernel’s\nmainline, which we accomplished by the end of the year. In total, I\nauthored 11 patches and co-authored 3 (all of which are already at\nTorvald’s repo). If you want to know more about my contributions to\nLinux Kernel, take a look at the Appendix section.\n\n## FLUSP: FLOSS at USP\n\nAfter the amazing experience contributing to the Linux Kernel, we\ndecided to found FLUSP: FLOSS at USP, a group opened to undergraduate\nand graduate students that aims to contribute to FLOSS software. Since\nthen, the group has grown and evolved a lot: Currently, we have members\ncontributing to the Kernel, GCC, IGT GPU Tools, Git and some projects of\nour own such as KernelWorkflow. And as a recognition of our endeavor\nwith free software, we received some donations from AnalogDevices and\nDigitalOcean.\n\nBesides administrative questions and contributions to FLOSS projects, at\nFLUSP, I’ve been mentoring people who want to start contributing to the\nLinux Kernel and now, to Git, as well.\n\n# About me and Git\n\nI joined Git community in February and, so far, I have sent the\nfollowing patches:\n\n        clone: test for our behavior on odd objects/* content\n        clone: better handle symlinked files at .git/objects/\n        dir-iterator: add flags parameter to dir_iterator_begin\n        clone: copy hidden paths at local clone\n        clone: extract function from copy_or_link_directory\n        clone: use dir-iterator to avoid explicit dir traversal\n        clone: Replace strcmp by fspathcmp\n\nAnd three more patches for git.github.io:\n\n        rn-50: Add git-send-email links to light readings\n        SoC-2019-Microprojects: Remove git-credential-cache\n        SoC-2019-Microprojects: Remove all trailing spaces\n\nParticipating at FLUSP, I’ve also been part of some Git related activities:\n\n* I actively helped to organize a Git workshop for newcomer students.\n* I’ve written an article at our website to help people configure and\nuse git-send-email to send patches.\n* I’ve been writing a ‘First steps at Git’ article (not finished yet),\nin which I’m registering what I’ve learned in the Git community so far,\nsince downloading the source, subscribing to the mailings list and\njoining the channel at IRC until how to use travis-ci and begin sending\npatches.\n\n# The Project\n\nAs direct as possible, the goal with this project is to make more of\nGit’s codebase thread-safe, so that we can improve parallelism in\nvarious commands. The motivation behind this are the complaints from\ndevelopers experiencing slow Git commands when working with large\nrepositories[1], such as chromium and Android. And since nowadays, most\npersonal computers have multi-core CPUs, it is a natural step trying to\nimprove parallel support so that we can better use the available resources.\n\nWith this in mind, pack access code is a good target for improvement,\nsince it’s used by many Git commands (e.g., checkout, grep, blame, diff,\nlog, etc.). This section of the codebase is still sequential and has\nmany global states, which should be protected before we can work to\nimprove parallelism.\n\n## The Pack Access Code\n\nTo better describe what the pack access code is, we must talk about\nGit’s object storing (in a simplified way): Besides what are called\nloose objects, Git has a very optimized mechanism to compactly store\nobjects (blobs, trees, commits, etc.) in packfiles[2]. These files are\ncreated by[3]:\n\n1. listing objects;\n2. sorting the list with some good heuristics;\n3. traversing the list with a sliding window to find similar objects in\nthe window, in order to do delta decomposing;\n4. compress the objects with zlib and write them to the packfile.\n\nWhat we are calling pack access code in this document, is the set of\nfunctions responsible for retrieving the objects stored at the\npackfiles. This process consists, roughly speaking, in three parts:\n\n1. Locate and read the blob from packfile, using the index file;\n2. If the blob is a delta, locate and read the base object to apply the\ndelta on top of it;\n3. Once the full content is read, decompress it (using zlib inflate).\n\nNote: There is a delta cache for the second step so that if another\ndelta depends on the same base object, it is already in memory. This\ncache is global; also, the sliding windows, are global per packfile.\n\nIf these steps were thread-safe, the ability to perform the delta\nreconstruction (together with the delta cache lookup) and zlib inflation\nin parallel could bring a good speedup. At git-blame, for example,\n24%[4] of the time is spent in the call stack originated at\nread_object_file_extended. Not only this but once we have this big\nsection of the codebase thread-safe, we can work to parallelize even\nmore work at higher levels of the call stack. Therefore, with this\nproject, we aim to make room for many future optimizations in many Git\ncommands.\n\n# Plan\n\nI will probably be working mainly with packfile.c, sha1-file.c,\nobject-store.h, object.c and pack.h, however, I may also need to tackle\nother files. I will be focusing on the following three pack access call\nchains, found in git-grep and/or git-blame:\n\n\nread_object_file → repo_read_object_file → read_object_file_extended →\nread_object → oid_object_info_extended → find_pack_entry →\nfill_pack_entry → find_pack_entry_one → bsearch_pack and\nnth_packed_object_offset\n\noid_object_info → oid_object_info_extended → <same as previous>\n\nread_object_with_reference → read_object_file → <same as previous>\n\n\nIdeally, at the end of the project, it will be possible to call\nread_object_file, oid_object_info and read_object_with_reference with\nthread-safety, so that these operations can be, latter, performed in\nparallel.\n\nHere are some threads on Git’s mailing list where I started discussing\nmy project:\n\n* https://public-inbox.org/git/CAHd-oW7onvn4ugEjXzAX_OSVEfCboH3-FnGR00dU8iaoc+b8=Q@mail.gmail.com/\n* https://public-inbox.org/git/20190402005245.4983-1-matheus.bernardino@usp.br/#t\n\nAnd also, a previous attempt to make part of the pack access code\nthread-safe which I may use as a base:\n\n* https://public-inbox.org/git/20140212015727.1D63A403D3@wince.sfo.corp.google.com/#Z30builtin:gc.c\n\n# Points to work on\n\n* Investigate pack access call chains and look for non-thread-safe\noperations on then.\n* Protect packfile.c read-and-write global variables, such as\npack_open_windows, pack_open_fds and etc., using mutexes.\n* Just like the previous item, protect sha1-file.c global states such as\nthe object cache used by read_object_file(). (The object cache may be\nthread-local thought. This should still be studied.)\n* Investigate the delta cache, sliding pack window, and maybe other\nstates that should be protected as well.\n* Use GDB or GPROF to follow call chains inside pack access looking for\nfunctions with static variables in their scopes. These variables are\nthread-shared and should be protected or the functions to which they\nbelong be refactored.\n* Make sure tests cover functions I’ll be working on and refactor/add\ntests as needed\n* [Bonus] Once pack access is thread-safe, refactor the critical\nsections at git-grep to use more fine-grained mutexes. This will\nhopefully increase git-grep performance, especially in large repositories.\n* [Bonus] Check other mutex protected functions git-grep uses, not\nrelated with pack access, to see if we can implement a more\nfined-grained parallelism there. This functions are: fill_textconv,\nis_submodule_active, repo_submodule_init, repo_read_gitmodules and\nadd_to_alternates_memory.\n* [Bonus] Once pack access is thread-safe, ensure xdiff code used by\ngit-blame has thread-safety. I expect this to be easier.\n* [Bonus] If the previous bonus get completed, start discussing a\npossible parallel git-blame implementation with the community. We could\nwork a producer-consumer mechanism at blame.c’s assign_blame() function,\nfor a very good work sharing assignment (90% of git-blame’s time is\nspent here[5]). Or try threading at lower functions on the call stack\nthat still uses a lot of execution time such as the libxdiff ones.\n\n# Schedule\n\nThis is the planned schedule in which I should be working on. But I\nwould like to highlight that since there’s still a significant\ninvestigation period from now until early May, this can have some\nchanges or additions during the process.\n\nTimeline:\n\nInvestigation Time (Now - May, 5)\n     * Gather information of global states.\n     * Trace pack access call chain used by git commands like blame and\n  checkout.\n     * Try to classify which global variables are updated during pack\naccess call stack and, therefore, should be protected.\n     * Adjust the schedule as needed.\n\n\nCommunity Bounding and work on sha1-file.c global states (May, 6 - 26[6])\n     * Talk with the community about my then refined plan and ask for\ncomments.\n     * Protect object cache at sha1-file.c (or make it thread-local).\n     * Work on other sha1-file.c global states and non-thread-safe\nfunctions.\n\n\nWork on packfile.c global states (not including delta cache) (May, 27 -\nJune, 23)\n     * Conclude any unfinished work on sha1-file.c from the previous period.\n     * Protect packfile.c variables (pack_open_windows, pack_open_fds\nand etc.) and work on its non-protected functions.\n\n\nWork on delta cache and other global states (June, 24 - July, 21)\n     * Protect delta base cache operations (here we should study whether\nto add mutexes to the cache itself or to the underlying hashmap).\n     * Protect oid_* functions.\n     * Work on sliding window (this is the section I have less knowledge\nyet, so should be studied)\n\n\nWork on bonus and leftovers (July, 22 - August 19)\n     * This time will be reserved to finish any leftovers from the other\nperiods and, if we still have some spare time, work on the bonus items.\n     * Note: I also plan to attend DebConf from july 21th to 28th\n\n\n# Availability\n\nMy university vacations start on June 29, but since this is my last year\nand I’m attending just two courses plus the teaching assistance, I think\nit won’t be a problem. Also, the classes start back in early August, but\nI won’t have any more courses to attend in this next semester. I don’t\nhave any schedule trips besides DebConf, from July 21th to 28th (let me\nknow if any other Git community members plan to attend too, please). All\nchanges in availability will be communicated to the mentors in advance.\n\n# Project Relevance and after GSoC plans\n\nAs already pointed out, this project will make it feasible to improve\n(or add) parallelism in many Git commands. And that’s what I plan to do\nafter (or even during) GSoC, mainly with git-blame and git-grep. I’m\nalso trying to form a local community at FLUSP to keep contributing to Git.\n\nAppendix\n\nPatches at Linux Kernel\n\n        staging: iio: ad2s1210: fix 'assignment operator' style checks\n        staging:iio:ad2s90: Make read_raw return spi_read's error code\n        staging:iio:ad2s90: Make probe handle spi_setup failure\n        staging:iio:ad2s90: Remove always overwritten assignment\n        staging:iio:ad2s90: Move device registration to the end of probe\n        staging:iio:ad2s90: Add IIO_CHAN_INFO_SCALE to channel spec and read_raw\n        staging:iio:ad2s90: Check channel type at read_raw\n        staging:iio:ad2s90: Add device tree support\n        staging:iio:ad2s90: Remove spi setup that should be done via dt\n        staging:iio:ad2s90: Add max frequency check at probe\n        dt-bindings:iio:resolver: Add docs for ad2s90\n        staging:iio:ad2s90: Replace license text w/ SPDX identifier\n        staging:iio:ad2s90: Add comment to device state mutex\n        staging:iio:ad2s90: Move out of staging\n\n\n________________\n[1] Some of them can be seen here:\nhttps://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/oYe69KzyG_U\nhttps://bugs.chromium.org/p/git/issues/detail?id=18\nhttps://bugs.chromium.org/p/git/issues/detail?id=16\nhttps://code.fb.com/core-data/scaling-mercurial-at-facebook/\nhttps://public-inbox.org/git/CA+TurHgyUK5sfCKrK+3xY8AeOg0t66vEvFxX=JiA9wXww7eZXQ@mail.gmail.com/\nhttps://public-inbox.org/git/20140213014229.GE4582@vauxhall.crustytoothpaste.net/\nhttps://public-inbox.org/git/CACBZZX6A+35wGBYAYj7E9d4XwLby21TLbTh-zRX+fkSt_e2zeg@mail.gmail.com/\n[2] https://git-scm.com/book/en/v2/Git-Internals-Packfiles\n[3]\nhttps://github.com/git/git/blob/master/Documentation/technical/pack-heuristics.txt\n[4]  https://i.imgur.com/XmyJMuE.png\n[5] https://i.imgur.com/XmyJMuE.png\n[6] GSoC’s official start date\n"},{"id":"373375","messageId":"CAP8UFD0qeOaS8NBOaMjzDf_tWJrgkYSAOgn8D=4JER2atg3H8g@mail.gmail.com","threadId":"50890","inReplyTo":"CAHd-oW7KMrDJ-cyzk63oqW9-QVpag6fKnDp+Mo5bWxg1KfzY3g@mail.gmail.com","subject":"Re: [GSoC][RFC] Proposal: Make pack access code thread-safe","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2019-04-07T22:52:08Z","receivedAt":"2019-04-07T22:52:24Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"Hi Matheus\n\nOn Sun, Apr 7, 2019 at 10:48 PM Matheus Tavares Bernardino\n<matheus.bernardino@usp.br> wrote:\n>\n> This is my proposal for GSoC with the subject \"Make pack access code\n> thread-safe\".\n\nThanks!\n\n> I'm late in schedule but I would like to ask for your\n> comments on it. Any feedback will be highly appreciated.\n>\n> The \"rendered\" version can be seen here:\n> https://docs.google.com/document/d/1QXT3iiI5zjwusplcZNf6IbYc04-9diziVKdOGkTHeIU/edit?usp=sharing\n\nThanks for the link!\n\n> Besides administrative questions and contributions to FLOSS projects, at\n> FLUSP, I’ve been mentoring people who want to start contributing to the\n> Linux Kernel and now, to Git, as well.\n\nNice! Do you have links about that?\n\n> # The Project\n>\n> As direct as possible, the goal with this project is to make more of\n> Git’s codebase thread-safe, so that we can improve parallelism in\n> various commands. The motivation behind this are the complaints from\n> developers experiencing slow Git commands when working with large\n> repositories[1], such as chromium and Android. And since nowadays, most\n> personal computers have multi-core CPUs, it is a natural step trying to\n> improve parallel support so that we can better use the available resources.\n>\n> With this in mind, pack access code is a good target for improvement,\n> since it’s used by many Git commands (e.g., checkout, grep, blame, diff,\n> log, etc.). This section of the codebase is still sequential and has\n> many global states, which should be protected before we can work to\n> improve parallelism.\n\nI think it's better if global state can be made local or perhaps\nremoved, rather than protected (though of course that's not always\npossible).\n\n> ## The Pack Access Code\n>\n> To better describe what the pack access code is, we must talk about\n> Git’s object storing (in a simplified way):\n\nMaybe s/storing/storage/\n\n> Besides what are called loose objects,\n\ns/loose object/loose object files/\n\n> Git has a very optimized mechanism to compactly store\n> objects (blobs, trees, commits, etc.) in packfiles[2]. These files are\n> created by[3]:\n>\n> 1. listing objects;\n> 2. sorting the list with some good heuristics;\n> 3. traversing the list with a sliding window to find similar objects in\n> the window, in order to do delta decomposing;\n> 4. compress the objects with zlib and write them to the packfile.\n>\n> What we are calling pack access code in this document, is the set of\n> functions responsible for retrieving the objects stored at the\n> packfiles. This process consists, roughly speaking, in three parts:\n>\n> 1. Locate and read the blob from packfile, using the index file;\n> 2. If the blob is a delta, locate and read the base object to apply the\n> delta on top of it;\n> 3. Once the full content is read, decompress it (using zlib inflate).\n>\n> Note: There is a delta cache for the second step so that if another\n> delta depends on the same base object, it is already in memory. This\n> cache is global; also, the sliding windows, are global per packfile.\n\nYeah, but the sliding windows are used only when creating pack files,\nnot when reading them, right?\n\n> If these steps were thread-safe, the ability to perform the delta\n> reconstruction (together with the delta cache lookup) and zlib inflation\n> in parallel could bring a good speedup. At git-blame, for example,\n> 24%[4] of the time is spent in the call stack originated at\n> read_object_file_extended. Not only this but once we have this big\n> section of the codebase thread-safe, we can work to parallelize even\n> more work at higher levels of the call stack. Therefore, with this\n> project, we aim to make room for many future optimizations in many Git\n> commands.\n\nNice.\n\n> # Plan\n>\n> I will probably be working mainly with packfile.c, sha1-file.c,\n> object-store.h, object.c and pack.h, however, I may also need to tackle\n> other files. I will be focusing on the following three pack access call\n> chains, found in git-grep and/or git-blame:\n>\n> read_object_file → repo_read_object_file → read_object_file_extended →\n> read_object → oid_object_info_extended → find_pack_entry →\n> fill_pack_entry → find_pack_entry_one → bsearch_pack and\n> nth_packed_object_offset\n>\n> oid_object_info → oid_object_info_extended → <same as previous>\n>\n> read_object_with_reference → read_object_file → <same as previous>\n>\n> Ideally, at the end of the project, it will be possible to call\n> read_object_file, oid_object_info and read_object_with_reference with\n> thread-safety, so that these operations can be, latter, performed in\n> parallel.\n>\n> Here are some threads on Git’s mailing list where I started discussing\n> my project:\n>\n> * https://public-inbox.org/git/CAHd-oW7onvn4ugEjXzAX_OSVEfCboH3-FnGR00dU8iaoc+b8=Q@mail.gmail.com/\n> * https://public-inbox.org/git/20190402005245.4983-1-matheus.bernardino@usp.br/#t\n>\n> And also, a previous attempt to make part of the pack access code\n> thread-safe which I may use as a base:\n>\n> * https://public-inbox.org/git/20140212015727.1D63A403D3@wince.sfo.corp.google.com/#Z30builtin:gc.c\n\nNice.\n\n> # Points to work on\n>\n> * Investigate pack access call chains and look for non-thread-safe\n> operations on then.\n> * Protect packfile.c read-and-write global variables, such as\n> pack_open_windows, pack_open_fds and etc., using mutexes.\n\nDo you want to work on making both packfile reading and packfile\nwriting thread safe? Or just packfile reading?\n\nIf some variables are used for both reading and writing packfiles, do\nyou plan to protect them only when they are used for reading?\n\nThe rest of your proposal looks very good to me. Please make sure you\nupload this or an updated version soon to the GSoC web site.\n\nThanks,\nChristian.\n"},{"id":"373382","messageId":"CACsJy8DxW7ZcSNQBZq4+A6c+9xZopg79sXfi6Na61Xgcoqd6ng@mail.gmail.com","threadId":"50890","inReplyTo":"CAP8UFD0qeOaS8NBOaMjzDf_tWJrgkYSAOgn8D=4JER2atg3H8g@mail.gmail.com","subject":"Re: [GSoC][RFC] Proposal: Make pack access code thread-safe","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-04-08T01:23:26Z","receivedAt":"2019-04-08T01:23:56Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Apr 8, 2019 at 5:52 AM Christian Couder\n<christian.couder@gmail.com> wrote:\n> > Git has a very optimized mechanism to compactly store\n> > objects (blobs, trees, commits, etc.) in packfiles[2]. These files are\n> > created by[3]:\n> >\n> > 1. listing objects;\n> > 2. sorting the list with some good heuristics;\n> > 3. traversing the list with a sliding window to find similar objects in\n> > the window, in order to do delta decomposing;\n> > 4. compress the objects with zlib and write them to the packfile.\n> >\n> > What we are calling pack access code in this document, is the set of\n> > functions responsible for retrieving the objects stored at the\n> > packfiles. This process consists, roughly speaking, in three parts:\n> >\n> > 1. Locate and read the blob from packfile, using the index file;\n> > 2. If the blob is a delta, locate and read the base object to apply the\n> > delta on top of it;\n> > 3. Once the full content is read, decompress it (using zlib inflate).\n> >\n> > Note: There is a delta cache for the second step so that if another\n> > delta depends on the same base object, it is already in memory. This\n> > cache is global; also, the sliding windows, are global per packfile.\n>\n> Yeah, but the sliding windows are used only when creating pack files,\n> not when reading them, right?\n\nThese windows are actually for reading. We used to just mmap the whole\npack file in the early days but that was impossible for 4+ GB packs on\n32-bit platforms, which was one of the reasons, I think, that sliding\nwindows were added, to map just the parts we want to read.\n\n> > # Points to work on\n> >\n> > * Investigate pack access call chains and look for non-thread-safe\n> > operations on then.\n> > * Protect packfile.c read-and-write global variables, such as\n> > pack_open_windows, pack_open_fds and etc., using mutexes.\n>\n> Do you want to work on making both packfile reading and packfile\n> writing thread safe? Or just packfile reading?\n\nPackfile writing is probably already or pretty close to thread-safe\n(at least the main writing code path in git-pack-objects; the\nstreaming blobs to a pack, i'm not so sure).\n-- \nDuy\n"},{"id":"373386","messageId":"CACsJy8AWHfcx=aLChWB_pN9dsVEj0Kq31zHmVnN1iVjXxyZVLg@mail.gmail.com","threadId":"50890","inReplyTo":"CACsJy8DxW7ZcSNQBZq4+A6c+9xZopg79sXfi6Na61Xgcoqd6ng@mail.gmail.com","subject":"Re: [GSoC][RFC] Proposal: Make pack access code thread-safe","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2019-04-08T03:32:22Z","receivedAt":"2019-04-08T03:35:00Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Mon, Apr 8, 2019 at 8:23 AM Duy Nguyen <pclouds@gmail.com> wrote:\n>\n> On Mon, Apr 8, 2019 at 5:52 AM Christian Couder\n> <christian.couder@gmail.com> wrote:\n> > > Git has a very optimized mechanism to compactly store\n> > > objects (blobs, trees, commits, etc.) in packfiles[2]. These files are\n> > > created by[3]:\n> > >\n> > > 1. listing objects;\n> > > 2. sorting the list with some good heuristics;\n> > > 3. traversing the list with a sliding window to find similar objects in\n> > > the window, in order to do delta decomposing;\n> > > 4. compress the objects with zlib and write them to the packfile.\n> > >\n> > > What we are calling pack access code in this document, is the set of\n> > > functions responsible for retrieving the objects stored at the\n> > > packfiles. This process consists, roughly speaking, in three parts:\n> > >\n> > > 1. Locate and read the blob from packfile, using the index file;\n> > > 2. If the blob is a delta, locate and read the base object to apply the\n> > > delta on top of it;\n> > > 3. Once the full content is read, decompress it (using zlib inflate).\n> > >\n> > > Note: There is a delta cache for the second step so that if another\n> > > delta depends on the same base object, it is already in memory. This\n> > > cache is global; also, the sliding windows, are global per packfile.\n> >\n> > Yeah, but the sliding windows are used only when creating pack files,\n> > not when reading them, right?\n>\n> These windows are actually for reading. We used to just mmap the whole\n> pack file in the early days but that was impossible for 4+ GB packs on\n> 32-bit platforms, which was one of the reasons, I think, that sliding\n> windows were added, to map just the parts we want to read.\n\nTo clarify (I think I see why you mentioned pack creation now), there\nare actually two window concepts. core.packedGitWindowSize is about\nreading pack files. pack.window is for generating pack files. The\nsecond window should already be thread-safe since we do all the\nheuristics to find best base object candidates in threads.\n-- \nDuy\n"},{"id":"373402","messageId":"CAP8UFD3qRTUv0H3Bw=6MJ0qhF15sj6O0LSV_WCymqoWxVJ6YBQ@mail.gmail.com","threadId":"50890","inReplyTo":"CACsJy8AWHfcx=aLChWB_pN9dsVEj0Kq31zHmVnN1iVjXxyZVLg@mail.gmail.com","subject":"Re: [GSoC][RFC] Proposal: Make pack access code thread-safe","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2019-04-08T06:58:30Z","receivedAt":"2019-04-08T06:58:44Z","isPatch":false,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Apr 8, 2019 at 5:32 AM Duy Nguyen <pclouds@gmail.com> wrote:\n>\n> On Mon, Apr 8, 2019 at 8:23 AM Duy Nguyen <pclouds@gmail.com> wrote:\n> >\n> > On Mon, Apr 8, 2019 at 5:52 AM Christian Couder\n> > <christian.couder@gmail.com> wrote:\n> > > > Git has a very optimized mechanism to compactly store\n> > > > objects (blobs, trees, commits, etc.) in packfiles[2]. These files are\n> > > > created by[3]:\n> > > >\n> > > > 1. listing objects;\n> > > > 2. sorting the list with some good heuristics;\n> > > > 3. traversing the list with a sliding window to find similar objects in\n> > > > the window, in order to do delta decomposing;\n> > > > 4. compress the objects with zlib and write them to the packfile.\n> > > >\n> > > > What we are calling pack access code in this document, is the set of\n> > > > functions responsible for retrieving the objects stored at the\n> > > > packfiles. This process consists, roughly speaking, in three parts:\n> > > >\n> > > > 1. Locate and read the blob from packfile, using the index file;\n> > > > 2. If the blob is a delta, locate and read the base object to apply the\n> > > > delta on top of it;\n> > > > 3. Once the full content is read, decompress it (using zlib inflate).\n> > > >\n> > > > Note: There is a delta cache for the second step so that if another\n> > > > delta depends on the same base object, it is already in memory. This\n> > > > cache is global; also, the sliding windows, are global per packfile.\n> > >\n> > > Yeah, but the sliding windows are used only when creating pack files,\n> > > not when reading them, right?\n> >\n> > These windows are actually for reading. We used to just mmap the whole\n> > pack file in the early days but that was impossible for 4+ GB packs on\n> > 32-bit platforms, which was one of the reasons, I think, that sliding\n> > windows were added, to map just the parts we want to read.\n>\n> To clarify (I think I see why you mentioned pack creation now), there\n> are actually two window concepts. core.packedGitWindowSize is about\n> reading pack files. pack.window is for generating pack files. The\n> second window should already be thread-safe since we do all the\n> heuristics to find best base object candidates in threads.\n\nYeah, it is not very clear in the proposal which windows it is talking\nabout as I think a window is first mentioned when describing the steps\nto create a packfile in:\n\n\"3. traversing the list with a sliding window to find similar objects\nin the window, in order to do delta decomposing;\"\n\nAlso the proposal plans to \"Protect packfile.c read-and-write global\nvariables ...\" which made me wonder if it was also about improving\nthread safety when generating pack files.\n\nThanks for clarifying!\n"},{"id":"373405","messageId":"79ecdc5b-2ccf-ae4d-3775-b850641f8c3e@iee.org","threadId":"50890","inReplyTo":"CACsJy8DxW7ZcSNQBZq4+A6c+9xZopg79sXfi6Na61Xgcoqd6ng@mail.gmail.com","subject":"Re: [GSoC][RFC] Proposal: Make pack access code thread-safe","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2019-04-08T09:26:55Z","receivedAt":"2019-04-08T09:27:00Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 08/04/2019 02:23, Duy Nguyen wrote:\n> On Mon, Apr 8, 2019 at 5:52 AM Christian Couder\n> <christian.couder@gmail.com> wrote:\n>>> Git has a very optimized mechanism to compactly store\n>>> objects (blobs, trees, commits, etc.) in packfiles[2]. These files are\n>>> created by[3]:\n>>>\n>>> 1. listing objects;\n>>> 2. sorting the list with some good heuristics;\n>>> 3. traversing the list with a sliding window to find similar objects in\n>>> the window, in order to do delta decomposing;\n>>> 4. compress the objects with zlib and write them to the packfile.\n>>>\n>>> What we are calling pack access code in this document, is the set of\n>>> functions responsible for retrieving the objects stored at the\n>>> packfiles. This process consists, roughly speaking, in three parts:\n>>>\n>>> 1. Locate and read the blob from packfile, using the index file;\n>>> 2. If the blob is a delta, locate and read the base object to apply the\n>>> delta on top of it;\n>>> 3. Once the full content is read, decompress it (using zlib inflate).\n>>>\n>>> Note: There is a delta cache for the second step so that if another\n>>> delta depends on the same base object, it is already in memory. This\n>>> cache is global; also, the sliding windows, are global per packfile.\n>> Yeah, but the sliding windows are used only when creating pack files,\n>> not when reading them, right?\n> These windows are actually for reading. We used to just mmap the whole\n> pack file in the early days but that was impossible for 4+ GB packs on\n> 32-bit platforms, which was one of the reasons, I think, that sliding\n> windows were added, to map just the parts we want to read.\n\nAnother \"32-bit problem\" should also be expressly considered during the \nGSoC work because of the MS Windows definition of uInt / long to be only \n32 bits, leading to much of the Git code failing on the Git for Windows \nport and on the Git LFS (for Windows) for packs and files greater than \n4Gb. https://github.com/git-for-windows/git/issues/1063\n\nMainly it is just substitution of size_t for long, but there can be \nunexpected coercions when mixed data types get coerced down to a local \n32-bit long. This is made worse by it being implementation defined, so \none needs to be explicit about some casts up to pointer/memsized types.\n>>> # Points to work on\n>>>\n>>> * Investigate pack access call chains and look for non-thread-safe\n>>> operations on then.\n>>> * Protect packfile.c read-and-write global variables, such as\n>>> pack_open_windows, pack_open_fds and etc., using mutexes.\n>> Do you want to work on making both packfile reading and packfile\n>> writing thread safe? Or just packfile reading?\n> Packfile writing is probably already or pretty close to thread-safe\n> (at least the main writing code path in git-pack-objects; the\n> streaming blobs to a pack, i'm not so sure).\n--\nPhilip\n"},{"id":"373429","messageId":"CAHd-oW6Cmn4TsR9GfwD0H8niwh-cyaXU-V8GK70j6bLT94_VkQ@mail.gmail.com","threadId":"50890","inReplyTo":"CACsJy8AWHfcx=aLChWB_pN9dsVEj0Kq31zHmVnN1iVjXxyZVLg@mail.gmail.com","subject":"Re: [GSoC][RFC] Proposal: Make pack access code thread-safe","fromName":"Matheus Tavares Bernardino","fromEmail":"matheus.bernardino@usp.br","sentAt":"2019-04-08T15:58:02Z","receivedAt":"2019-04-08T15:58:16Z","isPatch":false,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"On Mon, Apr 8, 2019 at 12:32 AM Duy Nguyen <pclouds@gmail.com> wrote:\n>\n> On Mon, Apr 8, 2019 at 8:23 AM Duy Nguyen <pclouds@gmail.com> wrote:\n> >\n> > On Mon, Apr 8, 2019 at 5:52 AM Christian Couder\n> > <christian.couder@gmail.com> wrote:\n> > > > Git has a very optimized mechanism to compactly store\n> > > > objects (blobs, trees, commits, etc.) in packfiles[2]. These files are\n> > > > created by[3]:\n> > > >\n> > > > 1. listing objects;\n> > > > 2. sorting the list with some good heuristics;\n> > > > 3. traversing the list with a sliding window to find similar objects in\n> > > > the window, in order to do delta decomposing;\n> > > > 4. compress the objects with zlib and write them to the packfile.\n> > > >\n> > > > What we are calling pack access code in this document, is the set of\n> > > > functions responsible for retrieving the objects stored at the\n> > > > packfiles. This process consists, roughly speaking, in three parts:\n> > > >\n> > > > 1. Locate and read the blob from packfile, using the index file;\n> > > > 2. If the blob is a delta, locate and read the base object to apply the\n> > > > delta on top of it;\n> > > > 3. Once the full content is read, decompress it (using zlib inflate).\n> > > >\n> > > > Note: There is a delta cache for the second step so that if another\n> > > > delta depends on the same base object, it is already in memory. This\n> > > > cache is global; also, the sliding windows, are global per packfile.\n> > >\n> > > Yeah, but the sliding windows are used only when creating pack files,\n> > > not when reading them, right?\n> >\n> > These windows are actually for reading. We used to just mmap the whole\n> > pack file in the early days but that was impossible for 4+ GB packs on\n> > 32-bit platforms, which was one of the reasons, I think, that sliding\n> > windows were added, to map just the parts we want to read.\n>\n> To clarify (I think I see why you mentioned pack creation now), there\n> are actually two window concepts. core.packedGitWindowSize is about\n> reading pack files. pack.window is for generating pack files. The\n> second window should already be thread-safe since we do all the\n> heuristics to find best base object candidates in threads.\n\nI was indeed confusing this two concepts, thanks for clarifying it! I\ntook a quick look around the usage of core.packedGitWindowSize arround\nthe code (at packfile.c) and it seems to be already thread-safe (I may\nbe wrong thought).\n\n> --\n> Duy\n"},{"id":"373430","messageId":"CAHd-oW4EAiY_FuoyBrHs2B=6NBo2_6grcq+_JiSJuA_-xTQp6w@mail.gmail.com","threadId":"50890","inReplyTo":"CAP8UFD3qRTUv0H3Bw=6MJ0qhF15sj6O0LSV_WCymqoWxVJ6YBQ@mail.gmail.com","subject":"Re: [GSoC][RFC] Proposal: Make pack access code thread-safe","fromName":"Matheus Tavares Bernardino","fromEmail":"matheus.bernardino@usp.br","sentAt":"2019-04-08T16:03:11Z","receivedAt":"2019-04-08T16:03:24Z","isPatch":false,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"On Mon, Apr 8, 2019 at 3:58 AM Christian Couder\n<christian.couder@gmail.com> wrote:\n>\n> On Mon, Apr 8, 2019 at 5:32 AM Duy Nguyen <pclouds@gmail.com> wrote:\n> >\n> > On Mon, Apr 8, 2019 at 8:23 AM Duy Nguyen <pclouds@gmail.com> wrote:\n> > >\n> > > On Mon, Apr 8, 2019 at 5:52 AM Christian Couder\n> > > <christian.couder@gmail.com> wrote:\n> > > > > Git has a very optimized mechanism to compactly store\n> > > > > objects (blobs, trees, commits, etc.) in packfiles[2]. These files are\n> > > > > created by[3]:\n> > > > >\n> > > > > 1. listing objects;\n> > > > > 2. sorting the list with some good heuristics;\n> > > > > 3. traversing the list with a sliding window to find similar objects in\n> > > > > the window, in order to do delta decomposing;\n> > > > > 4. compress the objects with zlib and write them to the packfile.\n> > > > >\n> > > > > What we are calling pack access code in this document, is the set of\n> > > > > functions responsible for retrieving the objects stored at the\n> > > > > packfiles. This process consists, roughly speaking, in three parts:\n> > > > >\n> > > > > 1. Locate and read the blob from packfile, using the index file;\n> > > > > 2. If the blob is a delta, locate and read the base object to apply the\n> > > > > delta on top of it;\n> > > > > 3. Once the full content is read, decompress it (using zlib inflate).\n> > > > >\n> > > > > Note: There is a delta cache for the second step so that if another\n> > > > > delta depends on the same base object, it is already in memory. This\n> > > > > cache is global; also, the sliding windows, are global per packfile.\n> > > >\n> > > > Yeah, but the sliding windows are used only when creating pack files,\n> > > > not when reading them, right?\n> > >\n> > > These windows are actually for reading. We used to just mmap the whole\n> > > pack file in the early days but that was impossible for 4+ GB packs on\n> > > 32-bit platforms, which was one of the reasons, I think, that sliding\n> > > windows were added, to map just the parts we want to read.\n> >\n> > To clarify (I think I see why you mentioned pack creation now), there\n> > are actually two window concepts. core.packedGitWindowSize is about\n> > reading pack files. pack.window is for generating pack files. The\n> > second window should already be thread-safe since we do all the\n> > heuristics to find best base object candidates in threads.\n>\n> Yeah, it is not very clear in the proposal which windows it is talking\n> about as I think a window is first mentioned when describing the steps\n> to create a packfile in:\n>\n> \"3. traversing the list with a sliding window to find similar objects\n> in the window, in order to do delta decomposing;\"\n>\n> Also the proposal plans to \"Protect packfile.c read-and-write global\n> variables ...\" which made me wonder if it was also about improving\n> thread safety when generating pack files.\n\nSorry, it is indeed unclear. The idea here was to say that variables\nwhich are both read and updated in code that must be thread-safe\nshould be protected. I will refactor this, thanks.\n\nOh, also I'm targeting just packfile reading. The explanation on how\npackfiles are created was written just as a contextualization. But\nperhaps it leaded to some confusion on the proposal's objective.\nThanks for this feedback too.\n\n> Thanks for clarifying!\n"},{"id":"373433","messageId":"CAHd-oW7UQdmyeODUOPdjMnmumFaQZ7_bzB2-x=crN92arSOejQ@mail.gmail.com","threadId":"50890","inReplyTo":"CAP8UFD0qeOaS8NBOaMjzDf_tWJrgkYSAOgn8D=4JER2atg3H8g@mail.gmail.com","subject":"Re: [GSoC][RFC] Proposal: Make pack access code thread-safe","fromName":"Matheus Tavares Bernardino","fromEmail":"matheus.bernardino@usp.br","sentAt":"2019-04-08T16:42:17Z","receivedAt":"2019-04-08T16:42:31Z","isPatch":false,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"On Sun, Apr 7, 2019 at 7:52 PM Christian Couder\n<christian.couder@gmail.com> wrote:\n>\n> Hi Matheus\n>\n> On Sun, Apr 7, 2019 at 10:48 PM Matheus Tavares Bernardino\n> <matheus.bernardino@usp.br> wrote:\n> >\n> > This is my proposal for GSoC with the subject \"Make pack access code\n> > thread-safe\".\n>\n> Thanks!\n>\n> > I'm late in schedule but I would like to ask for your\n> > comments on it. Any feedback will be highly appreciated.\n> >\n> > The \"rendered\" version can be seen here:\n> > https://docs.google.com/document/d/1QXT3iiI5zjwusplcZNf6IbYc04-9diziVKdOGkTHeIU/edit?usp=sharing\n>\n> Thanks for the link!\n>\n> > Besides administrative questions and contributions to FLOSS projects, at\n> > FLUSP, I’ve been mentoring people who want to start contributing to the\n> > Linux Kernel and now, to Git, as well.\n>\n> Nice! Do you have links about that?\n\nUnfortunately not :( Maybe just the mentoring slides (e.g.\nhttps://flusp.ime.usp.br/materials/Kernel_Primeiros_Passos.pdf). But\nthey are all in Portuguese, so I don't know wether it would be\nvaluable to add them here...\n\n> > # The Project\n> >\n> > As direct as possible, the goal with this project is to make more of\n> > Git’s codebase thread-safe, so that we can improve parallelism in\n> > various commands. The motivation behind this are the complaints from\n> > developers experiencing slow Git commands when working with large\n> > repositories[1], such as chromium and Android. And since nowadays, most\n> > personal computers have multi-core CPUs, it is a natural step trying to\n> > improve parallel support so that we can better use the available resources.\n> >\n> > With this in mind, pack access code is a good target for improvement,\n> > since it’s used by many Git commands (e.g., checkout, grep, blame, diff,\n> > log, etc.). This section of the codebase is still sequential and has\n> > many global states, which should be protected before we can work to\n> > improve parallelism.\n>\n> I think it's better if global state can be made local or perhaps\n> removed, rather than protected (though of course that's not always\n> possible).\n\nIndeed! I just added this to the docs version. Thanks\n\n> > ## The Pack Access Code\n> >\n> > To better describe what the pack access code is, we must talk about\n> > Git’s object storing (in a simplified way):\n>\n> Maybe s/storing/storage/\n\nThanks. Already changed.\n\n> > Besides what are called loose objects,\n>\n> s/loose object/loose object files/\n\nDone, thanks!\n\n> > Git has a very optimized mechanism to compactly store\n> > objects (blobs, trees, commits, etc.) in packfiles[2]. These files are\n> > created by[3]:\n> >\n> > 1. listing objects;\n> > 2. sorting the list with some good heuristics;\n> > 3. traversing the list with a sliding window to find similar objects in\n> > the window, in order to do delta decomposing;\n> > 4. compress the objects with zlib and write them to the packfile.\n> >\n> > What we are calling pack access code in this document, is the set of\n> > functions responsible for retrieving the objects stored at the\n> > packfiles. This process consists, roughly speaking, in three parts:\n> >\n> > 1. Locate and read the blob from packfile, using the index file;\n> > 2. If the blob is a delta, locate and read the base object to apply the\n> > delta on top of it;\n> > 3. Once the full content is read, decompress it (using zlib inflate).\n> >\n> > Note: There is a delta cache for the second step so that if another\n> > delta depends on the same base object, it is already in memory. This\n> > cache is global; also, the sliding windows, are global per packfile.\n>\n> Yeah, but the sliding windows are used only when creating pack files,\n> not when reading them, right?\n>\n> > If these steps were thread-safe, the ability to perform the delta\n> > reconstruction (together with the delta cache lookup) and zlib inflation\n> > in parallel could bring a good speedup. At git-blame, for example,\n> > 24%[4] of the time is spent in the call stack originated at\n> > read_object_file_extended. Not only this but once we have this big\n> > section of the codebase thread-safe, we can work to parallelize even\n> > more work at higher levels of the call stack. Therefore, with this\n> > project, we aim to make room for many future optimizations in many Git\n> > commands.\n>\n> Nice.\n>\n> > # Plan\n> >\n> > I will probably be working mainly with packfile.c, sha1-file.c,\n> > object-store.h, object.c and pack.h, however, I may also need to tackle\n> > other files. I will be focusing on the following three pack access call\n> > chains, found in git-grep and/or git-blame:\n> >\n> > read_object_file → repo_read_object_file → read_object_file_extended →\n> > read_object → oid_object_info_extended → find_pack_entry →\n> > fill_pack_entry → find_pack_entry_one → bsearch_pack and\n> > nth_packed_object_offset\n> >\n> > oid_object_info → oid_object_info_extended → <same as previous>\n> >\n> > read_object_with_reference → read_object_file → <same as previous>\n> >\n> > Ideally, at the end of the project, it will be possible to call\n> > read_object_file, oid_object_info and read_object_with_reference with\n> > thread-safety, so that these operations can be, latter, performed in\n> > parallel.\n> >\n> > Here are some threads on Git’s mailing list where I started discussing\n> > my project:\n> >\n> > * https://public-inbox.org/git/CAHd-oW7onvn4ugEjXzAX_OSVEfCboH3-FnGR00dU8iaoc+b8=Q@mail.gmail.com/\n> > * https://public-inbox.org/git/20190402005245.4983-1-matheus.bernardino@usp.br/#t\n> >\n> > And also, a previous attempt to make part of the pack access code\n> > thread-safe which I may use as a base:\n> >\n> > * https://public-inbox.org/git/20140212015727.1D63A403D3@wince.sfo.corp.google.com/#Z30builtin:gc.c\n>\n> Nice.\n>\n> > # Points to work on\n> >\n> > * Investigate pack access call chains and look for non-thread-safe\n> > operations on then.\n> > * Protect packfile.c read-and-write global variables, such as\n> > pack_open_windows, pack_open_fds and etc., using mutexes.\n>\n> Do you want to work on making both packfile reading and packfile\n> writing thread safe? Or just packfile reading?\n\nI plan to work on packfile reading, only.\n\n> If some variables are used for both reading and writing packfiles, do\n> you plan to protect them only when they are used for reading?\n\nHm, I haven't thought of that before. But indeed, if they are used for\nboth, I think I should protect them in both cases.\n\n> The rest of your proposal looks very good to me. Please make sure you\n> upload this or an updated version soon to the GSoC web site.\n\nThanks, Christian. I will work on the final points today and submit it.\n\n> Thanks,\n> Christian.\n"},{"id":"373435","messageId":"CAHd-oW5PFmj4u2YB-1TcHWNxkokfCEsUq_zB=Rx2Vmdk5Z9eMw@mail.gmail.com","threadId":"50890","inReplyTo":"79ecdc5b-2ccf-ae4d-3775-b850641f8c3e@iee.org","subject":"Re: [GSoC][RFC] Proposal: Make pack access code thread-safe","fromName":"Matheus Tavares Bernardino","fromEmail":"matheus.bernardino@usp.br","sentAt":"2019-04-08T17:04:39Z","receivedAt":"2019-04-08T17:04:53Z","isPatch":false,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"On Mon, Apr 8, 2019 at 6:26 AM Philip Oakley <philipoakley@iee.org> wrote:\n>\n> On 08/04/2019 02:23, Duy Nguyen wrote:\n> > On Mon, Apr 8, 2019 at 5:52 AM Christian Couder\n> > <christian.couder@gmail.com> wrote:\n> >>> Git has a very optimized mechanism to compactly store\n> >>> objects (blobs, trees, commits, etc.) in packfiles[2]. These files are\n> >>> created by[3]:\n> >>>\n> >>> 1. listing objects;\n> >>> 2. sorting the list with some good heuristics;\n> >>> 3. traversing the list with a sliding window to find similar objects in\n> >>> the window, in order to do delta decomposing;\n> >>> 4. compress the objects with zlib and write them to the packfile.\n> >>>\n> >>> What we are calling pack access code in this document, is the set of\n> >>> functions responsible for retrieving the objects stored at the\n> >>> packfiles. This process consists, roughly speaking, in three parts:\n> >>>\n> >>> 1. Locate and read the blob from packfile, using the index file;\n> >>> 2. If the blob is a delta, locate and read the base object to apply the\n> >>> delta on top of it;\n> >>> 3. Once the full content is read, decompress it (using zlib inflate).\n> >>>\n> >>> Note: There is a delta cache for the second step so that if another\n> >>> delta depends on the same base object, it is already in memory. This\n> >>> cache is global; also, the sliding windows, are global per packfile.\n> >> Yeah, but the sliding windows are used only when creating pack files,\n> >> not when reading them, right?\n> > These windows are actually for reading. We used to just mmap the whole\n> > pack file in the early days but that was impossible for 4+ GB packs on\n> > 32-bit platforms, which was one of the reasons, I think, that sliding\n> > windows were added, to map just the parts we want to read.\n>\n> Another \"32-bit problem\" should also be expressly considered during the\n> GSoC work because of the MS Windows definition of uInt / long to be only\n> 32 bits, leading to much of the Git code failing on the Git for Windows\n> port and on the Git LFS (for Windows) for packs and files greater than\n> 4Gb. https://github.com/git-for-windows/git/issues/1063\n\nThanks for pointing it out. I didn't get it, thought, if your\nsuggestion was to also propose tackling this issue in this GSoC\nproject. Was it that? I read the link but it seems to be a kind of\nunrelated problem from what I'm planing to do with the pack access\ncode (which is tread-safety). I may have understood this wrongly,\nthough. Please, let me know if that's the case :)\n\n> Mainly it is just substitution of size_t for long, but there can be\n> unexpected coercions when mixed data types get coerced down to a local\n> 32-bit long. This is made worse by it being implementation defined, so\n> one needs to be explicit about some casts up to pointer/memsized types.\n> >>> # Points to work on\n> >>>\n> >>> * Investigate pack access call chains and look for non-thread-safe\n> >>> operations on then.\n> >>> * Protect packfile.c read-and-write global variables, such as\n> >>> pack_open_windows, pack_open_fds and etc., using mutexes.\n> >> Do you want to work on making both packfile reading and packfile\n> >> writing thread safe? Or just packfile reading?\n> > Packfile writing is probably already or pretty close to thread-safe\n> > (at least the main writing code path in git-pack-objects; the\n> > streaming blobs to a pack, i'm not so sure).\n> --\n> Philip\n"},{"id":"373437","messageId":"1348d823-9729-a5eb-2104-df7c4a41911d@iee.org","threadId":"50890","inReplyTo":"CAHd-oW5PFmj4u2YB-1TcHWNxkokfCEsUq_zB=Rx2Vmdk5Z9eMw@mail.gmail.com","subject":"Re: [GSoC][RFC] Proposal: Make pack access code thread-safe","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.org","sentAt":"2019-04-08T19:19:58Z","receivedAt":"2019-04-08T19:20:01Z","isPatch":false,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"Hi Matheus\n\nOn 08/04/2019 18:04, Matheus Tavares Bernardino wrote:\n>> Another \"32-bit problem\" should also be expressly considered during the\n>> GSoC work because of the MS Windows definition of uInt / long to be only\n>> 32 bits, leading to much of the Git code failing on the Git for Windows\n>> port and on the Git LFS (for Windows) for packs and files greater than\n>> 4Gb.https://github.com/git-for-windows/git/issues/1063\n\n> Thanks for pointing it out. I didn't get it, thought, if your\n> suggestion was to also propose tackling this issue in this GSoC\n> project. Was it that? I read the link but it seems to be a kind of\n> unrelated problem from what I'm planing to do with the pack access\n> code (which is tread-safety). I may have understood this wrongly,\n> though. Please, let me know if that's the case :)\n> \nThe main point was to avoid accidental regressions by re-introducing \nsimple 'longs' where memsized types were more appropriate.\n\nTorsten has already done a lot of work at \nhttps://github.com/tboegi/git/tree/tb.190402_1552_convert_size_t_only_git_master_181124_mk_size_t\n\nHTH\nPhilip\n(I'm off line for a few days)\n"},{"id":"373438","messageId":"CAHd-oW79P+Fq6OkdDa4Ly2mecuCM0tyS7jopON2_2rL-mf9RXQ@mail.gmail.com","threadId":"50890","inReplyTo":"1348d823-9729-a5eb-2104-df7c4a41911d@iee.org","subject":"Re: [GSoC][RFC] Proposal: Make pack access code thread-safe","fromName":"Matheus Tavares Bernardino","fromEmail":"matheus.bernardino@usp.br","sentAt":"2019-04-08T19:36:11Z","receivedAt":"2019-04-08T19:36:25Z","isPatch":false,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"On Mon, Apr 8, 2019 at 4:19 PM Philip Oakley <philipoakley@iee.org> wrote:\n>\n> Hi Matheus\n>\n> On 08/04/2019 18:04, Matheus Tavares Bernardino wrote:\n> >> Another \"32-bit problem\" should also be expressly considered during the\n> >> GSoC work because of the MS Windows definition of uInt / long to be only\n> >> 32 bits, leading to much of the Git code failing on the Git for Windows\n> >> port and on the Git LFS (for Windows) for packs and files greater than\n> >> 4Gb.https://github.com/git-for-windows/git/issues/1063\n>\n> > Thanks for pointing it out. I didn't get it, thought, if your\n> > suggestion was to also propose tackling this issue in this GSoC\n> > project. Was it that? I read the link but it seems to be a kind of\n> > unrelated problem from what I'm planing to do with the pack access\n> > code (which is tread-safety). I may have understood this wrongly,\n> > though. Please, let me know if that's the case :)\n> >\n> The main point was to avoid accidental regressions by re-introducing\n> simple 'longs' where memsized types were more appropriate.\n>\n> Torsten has already done a lot of work at\n> https://github.com/tboegi/git/tree/tb.190402_1552_convert_size_t_only_git_master_181124_mk_size_t\n\nGot it. Thanks, Philip!\n\n> HTH\n> Philip\n> (I'm off line for a few days)\n"},{"id":"373456","messageId":"bebc5bfe-0d84-d499-4dff-8f72b8b9f12b@web.de","threadId":"50890","inReplyTo":"CAHd-oW79P+Fq6OkdDa4Ly2mecuCM0tyS7jopON2_2rL-mf9RXQ@mail.gmail.com","subject":"Re: [GSoC][RFC] Proposal: Make pack access code thread-safe","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2019-04-09T05:54:49Z","receivedAt":"2019-04-09T05:54:55Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2019-04-08 21:36, Matheus Tavares Bernardino wrote:\n> On Mon, Apr 8, 2019 at 4:19 PM Philip Oakley <philipoakley@iee.org> wrote:\n>>\n>> Hi Matheus\n>>\n>> On 08/04/2019 18:04, Matheus Tavares Bernardino wrote:\n>>>> Another \"32-bit problem\" should also be expressly considered during the\n>>>> GSoC work because of the MS Windows definition of uInt / long to be only\n>>>> 32 bits, leading to much of the Git code failing on the Git for Windows\n>>>> port and on the Git LFS (for Windows) for packs and files greater than\n>>>> 4Gb.https://github.com/git-for-windows/git/issues/1063\n>>\n>>> Thanks for pointing it out. I didn't get it, thought, if your\n>>> suggestion was to also propose tackling this issue in this GSoC\n>>> project. Was it that? I read the link but it seems to be a kind of\n>>> unrelated problem from what I'm planing to do with the pack access\n>>> code (which is tread-safety). I may have understood this wrongly,\n>>> though. Please, let me know if that's the case :)\n>>>\n>> The main point was to avoid accidental regressions by re-introducing\n>> simple 'longs' where memsized types were more appropriate.\n>>\n>> Torsten has already done a lot of work at\n>> https://github.com/tboegi/git/tree/tb.190402_1552_convert_size_t_only_git_master_181124_mk_size_t\n>\n> Got it. Thanks, Philip!\n>\n>> HTH\n>> Philip\n>> (I'm off line for a few days)\n\nThanks for the reminder -\nI will probably send something out the next days/weeks.\n"}]}