git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 5/9] pack-bitmap: introduce bitmap_walk_contains()

From
Philip Oakley <philipoakley@iee.email>
Date
Oct 19, 2019, 20:15 UTC
Message-ID
<c1764a23-15d1-cbef-a675-0f4242bc41f4@iee.email>
In-Reply-To
<CAP8UFD2dLPguCnUhTu+F4SNPxFZ7YT5io0qXkVwvSNP-sA8Jxg@mail.gmail.com>
Hi Christian,
On 19/10/2019 19:55, Christian Couder wrote:
Show 30 quoted lines
> Hi Philip,
>
> On Sat, Oct 19, 2019 at 5:25 PM Philip Oakley <philipoakley@iee.email> wrote:
>> Hi Christian,
>> can I check one thing?
> Yeah, sure! Thanks for taking a look at my patches!
>
>> On 19/10/2019 11:35, Christian Couder wrote:
>>> +int bitmap_walk_contains(struct bitmap_index *bitmap_git,
>>> +                      struct bitmap *bitmap, const struct object_id *oid)
>>> +{
>>> +     int idx;
>> Excuse my ignorance here...
>>
>> For the case on Windows (int/long 32 bit), is this return value
>> guaranteed to be less than 2GiB, i.e. not a memory offset?
>>
>> I'm just thinking ahead to the resolution of the 4GiB file limit issue
>> on Git-for-Windows (https://github.com/git-for-windows/git/pull/2179)
> I understand your concern, unfortunately, below we have:
>
> idx = bitmap_position(bitmap_git, oid);
>
> and bitmap_position() returns an int at least since 3ae5fa0768
> (pack-bitmap: remove bitmap_git global variable, 2018-06-07)
>
> So I think the fix would be much more involved than just changing the
> type of the idx variable. It would likely involve modifying
> bitmap_position(), and thus would probably best be addressed in a
> separate patch series.

So, IIUC it is mem-sized, so I should at least note it and pay attention to it for my >4G series, which like you say is "much more involved than just"...

The patch to flip over all the affected locations is a bit humongous (big), plus it's a bit of a moving target...

Show 7 quoted lines
>>> +
>>> +     if (!bitmap)
>>> +             return 0;
>>> +
>>> +     idx = bitmap_position(bitmap_git, oid);
>>> +     return idx >= 0 && bitmap_get(bitmap, idx);
>>> +}
Philip
Previous: Christian CouderNext: Jeff King
Message 11 of 22 in “Rewrite packfile reuse code”
  1. 0/9 Rewrite packfile reuse codeChristian Couder, Oct 19, 2019
  2. 1/9 builtin/pack-objects: report reused packfile objectsChristian Couder, Oct 19, 2019
  3. 2/9 packfile: expose get_delta_base()Christian Couder, Oct 19, 2019
  4. 3/9 ewah/bitmap: introduce bitmap_word_alloc()Christian Couder, Oct 19, 2019
  5. Jonathan TanOct 22, 2019
  6. Christian CouderOct 26, 2019
  7. 4/9 pack-bitmap: don't rely on bitmap_git->reuse_objectsChristian Couder, Oct 19, 2019
  8. 5/9 pack-bitmap: introduce bitmap_walk_contains()Christian Couder, Oct 19, 2019
  9. Philip OakleyOct 19, 2019
  10. Christian CouderOct 19, 2019
  11. Philip OakleyOct 19, 2019
  12. Jeff KingOct 19, 2019
  13. 6/9 csum-file: introduce hashfile_total()Christian Couder, Oct 19, 2019
  14. 7/9 pack-objects: introduce pack.allowPackReuseChristian Couder, Oct 19, 2019
  15. 8/9 builtin/pack-objects: introduce obj_is_packed()Christian Couder, Oct 19, 2019
  16. 9/9 pack-objects: improve partial packfile reuseChristian Couder, Oct 19, 2019
  17. Philip OakleyOct 19, 2019
  18. Christian CouderOct 19, 2019
  19. Jeff KingOct 19, 2019
  20. Philip OakleyOct 20, 2019
  21. Jonathan TanOct 22, 2019
  22. Christian CouderOct 26, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.