New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

core: replace `MultiMap` with standard library container #521

Open

mkviatkovskii opened this issue Nov 11, 2021 · 0 comments

Labels

c++ Core Improvement Technical

Milestone

Member

mkviatkovskii commented Nov 11, 2021 •

edited

Loading

Motivation
Currently Indigo uses own-written implementation of MultiMap instead of standard one. It does not contain any unique API that could not be replaced with standard container. Also it uses red-black trees even if we don't need the order of the keys, so we could also achieve performance improvement here.
The implementation could be found at core/indigo-core/common/base_cpp/multimap.h.

ToDo

Find all places in code that use MultiMap<K, V>, check if K is trivially hashable and order of elements is required, and replace it with std::multimap<K, V> or std::unordered_multimap<K, V> depending on the situation.

The text was updated successfully, but these errors were encountered:

mkviatkovskii added Improvement Core Technical c++ labels

mkviatkovskii added this to the indigo-1.6 milestone

mkviatkovskii changed the title ~~core: replace MultiMap with standard library container~~ core: replace MultiMap with standard library container

AlexanderSavelyev modified the milestones: indigo-1.7, indigo-1.8

AlexanderSavelyev modified the milestones: indigo-1.8, indigo-1.9

loimu pushed a commit that referenced this issue


          Issue #521: core: replace MultiMap with standard library container

a826052

Partial implementation: replace MultiMap in MolfileLoader class
Implementation details: tested against both std::multimap and std::unordered_multimap. std::multimap implementation is by a small fraction but faster than std::unordered_multimap counterpart

loimu pushed a commit that referenced this issue


          Issue #521: core: replace MultiMap with standard library container

4d12525

Partial implementation: replace MultiMap in MoleculeRGroupsComposition class

loimu pushed a commit that referenced this issue


          Issue #521: core: replace MultiMap with standard library container

e12c272

Partial implementation: replace MultiMap in MoleculeRGroupsComposition class

MysterionRise added a commit that referenced this issue


          Merge pull request #917 from epam/feature/521_std_multimap_molecule_r…

b0851a8

…groups_composition

Issue #521: core: replace MultiMap in MoleculeRGroupsComposition class

MysterionRise added a commit that referenced this issue


          Merge pull request #911 from epam/feature/521_std_multimap_molfile_lo…

1d1e43c

…ader

Issue #521: core: replace MultiMap  in MolfileLoader class

loimu pushed a commit that referenced this issue


          Issue #521: core: replace MultiMap with standard library container

5564e3e

Partial implementation: replace MultiMap in MoleculeRGroupsComposition class - Part 2

loimu pushed a commit that referenced this issue


          Issue #521: core: replace MultiMap with standard library container

867d225

Partial implementation: remove MultiMap implementation from the project completely. Fixes #521

loimu pushed a commit that referenced this issue


          Issue #521: core: replace MultiMap with standard library container

ab14bf8

Partial implementation: replace MultiMap in MolfileLoader class
Implementation details: tested against both std::multimap and std::unordered_multimap. std::multimap implementation is by a small fraction but faster than std::unordered_multimap counterpart

loimu pushed a commit that referenced this issue


          Issue #521: core: replace MultiMap with standard library container

764f4e0

Partial implementation: replace MultiMap in MoleculeRGroupsComposition class - Part 2

loimu pushed a commit that referenced this issue


          Issue #521: core: replace MultiMap with standard library container

62151d8

Partial implementation: remove MultiMap implementation from the project completely. Fixes #521

loimu pushed a commit that referenced this issue


          Issue #521: core: replace MultiMap with standard library container

572b4b2

Partial implementation: replace MultiMap in MoleculeRGroupsComposition class - Part 2

loimu pushed a commit that referenced this issue


          Issue #521: core: replace MultiMap with standard library container

4a2a8ca

Partial implementation: remove MultiMap implementation from the project completely. Fixes #521

loimu pushed a commit that referenced this issue


          Issue #521: core: replace MultiMap with standard library container

d119d96

Partial implementation: replace MultiMap in MoleculeRGroupsComposition class - Part 2

loimu pushed a commit that referenced this issue


          Issue #521: core: replace MultiMap with standard library container

Partial implementation: remove MultiMap implementation from the project completely. Fixes #521

loimu pushed a commit that referenced this issue


          Issue #521: core: replace MultiMap with standard library container

1dea10a

Partial implementation: replace MultiMap in MoleculeRGroupsComposition class - Part 2

loimu pushed a commit that referenced this issue


          Issue #521: core: replace MultiMap with standard library container

7fc95c0

Partial implementation: remove MultiMap implementation from the project completely. Fixes #521

loimu pushed a commit that referenced this issue


          Issue #521: core: replace MultiMap with standard library container

e1c3293

Replace std::multimap with std::map of vectors. Fixes randomly failing composition/basic test due to incorrect ordering of elements

loimu mentioned this issue

Fix randomly failing composition/basic test due to an incorrect ordering of fragments #923

Closed

loimu pushed a commit that referenced this issue


          Issue #521: core: replace MultiMap with standard library container

9379bb7

Replace std::multimap with std::map of vectors. Fixes randomly failing composition/basic test due to an incorrect ordering of fragments

loimu pushed a commit that referenced this issue


          Issue #521: core: replace MultiMap with standard library container

71e469b

Replace std::multimap with std::map of vectors. Fixes randomly failing composition/basic test due to an incorrect ordering of fragments

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment