Skip to content

remove unnecessary operation in long_compare() #79877

Closed
Closed
remove unnecessary operation in long_compare()#79877
Labels
3.9 (EOL)end of lifeinterpreter-core(Objects, Python, Grammar, and Parser dirs)pendingThe issue will be closed if no feedback is provided

Description

@animalize
animalize
mannequin
opened on Jan 9, 2019
BPO 35696
Nosy @vstinner, @methane, @animalize, @sir-sigurd, @hongweipeng
PRs
  • bpo-35696: some minor optimizations on hot path #11473
  • bpo-35696: some minor optimizations on hot path #11473
  • bpo-35696: some minor optimizations on hot path #11473
  • bpo-35696: Remove unnecessary operation in long_compare() #16146
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
closed_at = None
created_at = <Date 2019-01-09.09:26:39.577>
labels = ['3.9']
title = 'remove unnecessary operation in long_compare()'
updated_at = <Date 2019-09-21.09:56:56.010>
user = 'https://github.com/animalize'

    bugs.python.org fields:

    activity = <Date 2019-09-21.09:56:56.010>
actor = 'hongweipeng'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = []
creation = <Date 2019-01-09.09:26:39.577>
creator = 'malin'
dependencies = []
files = []
hgrepos = []
issue_num = 35696
keywords = ['patch', 'patch', 'patch']
message_count = 5.0
messages = ['333293', '352732', '352875', '352883', '352903']
nosy_count = 5.0
nosy_names = ['vstinner', 'methane', 'malin', 'sir-sigurd', 'hongweipeng']
pr_nums = ['11473', '11473', '11473', '16146']
priority = 'normal'
resolution = None
stage = 'resolved'
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue35696'
versions = ['Python 3.9']

    Activity

    1. animalize commented on Jan 9, 2019

      @animalize
      animalizemannequin
      on Jan 9, 2019
      MannequinAuthor
      static int
long_compare(PyLongObject *a, PyLongObject *b)
{
    ....
}

      This function in /Objects/longobject.c is used to compare two PyLongObject's value.
      We only need the sign, converting to -1 or +1 is not necessary.

    2. animalize
      added
      3.8 (EOL)end of life
      on Jan 9, 2019

    3. methane commented on Sep 18, 2019

      @methane
      methane
      on Sep 18, 2019
      Member

      New changeset 42acb7b by Inada Naoki (HongWeipeng) in branch 'master':
      bpo-35696: Simplify long_compare() (GH-16146)
      42acb7b

    4. methane
      added
      3.9 (EOL)end of life
      and removed
      3.8 (EOL)end of life
      on Sep 18, 2019
    5. methane
      closed this as completedon Sep 18, 2019

    6. vstinner commented on Sep 20, 2019

      @vstinner
      vstinner
      on Sep 20, 2019
      Member

      I reopen the issue. The win32 job of Azure Pipelines now logs a compiler warning:

      Objects\longobject.c(412,5): warning C4244: 'function': conversion from 'unsigned long' to 'sdigit', possible loss of data
      Objects\longobject.c(420,5): warning C4244: 'function': conversion from 'unsigned __int64' to 'sdigit', possible loss of data

      See also discussion on PR 16146:
      #16146 (comment)

    7. vstinner
      reopened this on Sep 20, 2019

    8. sir-sigurd commented on Sep 20, 2019

      @sir-sigurd
      sir-sigurdmannequin
      on Sep 20, 2019
      Mannequin

      These warnings are caused by c6734ee.

      I'd fix them, but I'm not sure if we are going to restore CHECK_SMALL_INT() ¯\(ツ)/¯

    9. animalize commented on Sep 20, 2019

      @animalize
      animalizemannequin
      on Sep 20, 2019
      MannequinAuthor

      I'd fix them, but I'm not sure if we are going to restore CHECK_SMALL_INT() ¯\(ツ)/¯

      I suggest we slow down, carefully sort out the recent commits for longobject.c:
      https://bugs.python.org/issue37812#msg352837

      Make the code has consistent style, better readability...

    10. ezio-melotti
      transferred this issue fromon Apr 10, 2022

    11. iritkatriel commented on Nov 27, 2023

      @iritkatriel
      iritkatriel
      on Nov 27, 2023
      Member

      Is there anything left to do here?

    12. iritkatriel
      added
      interpreter-core(Objects, Python, Grammar, and Parser dirs)
      pendingThe issue will be closed if no feedback is provided
      on Nov 27, 2023

    13. vstinner commented on Nov 27, 2023

      @vstinner
      vstinner
      on Nov 27, 2023
      Member

      I looked at Git history and it seems like I fixed the compiler warning with commit 6314abc:

      commit 6314abcc08f5d0f3d3a915dc9455ea223fa65517
Author: Victor Stinner <[email protected]>
Date:   Tue Oct 1 13:29:53 2019 +0200

    bpo-37802: Fix a compiler warning in longobject.c (GH-16517)
    
    bpo-37802, bpo-38321: Fix the following warnings:
    
        longobject.c(420): warning C4244: 'function': conversion from
        'unsigned __int64' to 'sdigit', possible loss of data
    
        longobject.c(428): warning C4267: 'function': conversion from
        'size_t' to 'sdigit', possible loss of data

      Thanks for the reminder @iritkatriel, I closed the issue.

    14. vstinner
      closed this as completedon Nov 27, 2023
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      3.9 (EOL)end of lifeinterpreter-core(Objects, Python, Grammar, and Parser dirs)pendingThe issue will be closed if no feedback is provided

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions